Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds new UI pages and modal flows for creating proposals and submitting evidence, a RightSidebar with widgets, numerous new client components (forms, profile cards, activity widgets), a treasury GET API endpoint with Substrate RPC logic, theme/layout updates, defensive utility fixes, and three UI dependencies. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Page as Create Proposal Page/Modal
participant Form as CreateProposalForm
User->>Page: Open Create Proposal
alt Not logged in
Page-->>User: Show login prompt/link
else Logged in
Page->>Form: Render with formRef
User->>Page: Click Create
Page->>Form: formRef.requestSubmit()
Form->>Form: Validate gates (login, API, address, fellow)
alt Valid
Form->>Chain: submit/simulate tx
Chain-->>Form: success
Form-->>Page: onSuccess()
Page-->>User: Close modal / show success
else Invalid/Error
Form-->>User: Show errors / update form state
Form-->>Page: onFormStateChange(isValid, isLoading)
end
end
sequenceDiagram
autonumber
actor UIUser
participant UI as RightSidebar
participant API as /api/v1/treasury
participant RPC as Substrate RPC (WsProvider + ApiPromise)
UI->>API: GET /api/v1/treasury (x-network)
API->>API: validate network header
API->>RPC: connect & await isReady
alt treasury pallet exists
API->>RPC: query spendPeriod, block, params
API->>API: compute cycle, progress, payout times
else fallback via fellowshipSalary
API->>RPC: query fellowship params & registrationPeriod
API->>API: derive cycle metrics
end
API->>RPC: disconnect
API-->>UI: 200 JSON metrics or APIError
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (4)
package.json (1)
34-34: Consider migration path for deprecated package.The
@nextui-org/selectpackage has been deprecated by its maintainers in favor of@heroui/select. While version 2.4.9 will continue to work, consider planning a migration to the recommended replacement package to ensure long-term maintainability and continued support.Based on learnings
src/components/Header/RightSidebar/QuickActions.tsx (2)
17-25: Consider adding aria-label for accessibility.While the button has visible text, adding an
aria-labelwould improve screen reader support, especially since the button combines an icon and text in a column layout.<Button variant='bordered' className='font-poppins flex h-16 flex-col gap-1 border-primary_border text-text_secondary hover:bg-gray-50' as={LinkWithNetwork} href='/submit-evidence' startContent={<FileText className='h-4 w-4' />} + aria-label='Submit Evidence' > <span className='text-xs'>Submit Evidence</span> </Button>Apply similar changes to other buttons in the grid.
35-41: Disable or implement functionality for the “Induct” button
The “Induct” button has no click handler or navigation—either add anonClick(or wrap withas={LinkWithNetwork}/href) or disable it (e.g.isDisabled+title="Coming soon").src/app/page.tsx (1)
9-9: Consider removing commented-out code.The commented imports and code blocks for
TrendingProposalsandgetTrendingProposalsare no longer needed since this functionality has been moved to theRightSidebarcomponent. Commented code can clutter the codebase and cause confusion for future maintainers.Apply this diff to remove the commented code:
-// import TrendingProposals from '@/components/Home/TrendingProposals'; import { API_ERROR_CODE } from '@/global/constants/errorCodes';import PendingTasks from '@/components/Home/PendingTasks'; import getActivityFeed from './api/v1/feed/getActivityFeed'; -// import getTrendingProposals from './api/v1/feed/trending/getTrendingProposals';const feedItems = await getActivityFeed({ feedType: feed as EActivityFeed, originUrl, network: network as Network }); - -// const trending = await getTrendingProposals({ originUrl, network: network as Network });{feed === EActivityFeed.ALL ? <ActivityFeed items={(feedItems || []) as ActivityFeedItem[]} /> : <PostFeed items={(feedItems || []) as PostFeedListingItem[]} />} </div> - {/* <div className='flex w-full flex-col gap-y-4 lg:w-[300px]'> - <PendingTasks className='hidden md:flex' /> - <Stats className='hidden lg:flex' /> - <TrendingProposals proposals={trending} /> - </div> */} </div> );Also applies to: 20-20, 46-46, 59-63
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
public/icons/user-group.svgis excluded by!**/*.svgyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (21)
package.json(3 hunks)src/app/(site)/create-proposal/page.tsx(1 hunks)src/app/(site)/submit-evidence/page.tsx(1 hunks)src/app/@modal/(site)/(.)create-proposal/page.tsx(1 hunks)src/app/@modal/(site)/(.)submit-evidence/page.tsx(1 hunks)src/app/api/v1/treasury/route.ts(1 hunks)src/app/layout.tsx(2 hunks)src/app/page.tsx(3 hunks)src/components/CreateProposal/CreateProposalForm.tsx(1 hunks)src/components/Header/AppSidebar.tsx(7 hunks)src/components/Header/Header.module.scss(1 hunks)src/components/Header/RightSidebar/ActivityFeed.tsx(1 hunks)src/components/Header/RightSidebar/QuickActions.tsx(1 hunks)src/components/Header/RightSidebar/RightSidebar.tsx(1 hunks)src/components/Header/RightSidebar/Treasury.tsx(1 hunks)src/components/Misc/AddressSwitch.tsx(1 hunks)src/components/Misc/EvidenceSelector.tsx(1 hunks)src/components/SubmitEvidence/SubmitEvidenceForm.tsx(1 hunks)src/global/globals.scss(1 hunks)src/global/themeColors.ts(3 hunks)tailwind.config.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (13)
src/app/(site)/submit-evidence/page.tsx (1)
src/contexts/index.tsx (1)
useUserDetailsContext(28-28)
src/app/(site)/create-proposal/page.tsx (1)
src/contexts/index.tsx (1)
useUserDetailsContext(28-28)
src/components/SubmitEvidence/SubmitEvidenceForm.tsx (1)
src/utils/getSubstrateAddress.ts (1)
getSubstrateAddress(13-21)
src/components/Header/RightSidebar/ActivityFeed.tsx (3)
src/global/types.ts (1)
ActivityFeedItem(660-678)src/utils/getOriginUrl.ts (1)
getOriginUrl(7-12)src/app/api/v1/feed/getActivityFeed.ts (1)
getActivityFeed(20-34)
src/components/CreateProposal/CreateProposalForm.tsx (2)
src/contexts/index.tsx (2)
useApiContext(28-28)useUserDetailsContext(28-28)src/utils/getSubstrateAddress.ts (1)
getSubstrateAddress(13-21)
src/components/Misc/AddressSwitch.tsx (1)
src/utils/getSubstrateAddress.ts (1)
getSubstrateAddress(13-21)
src/app/layout.tsx (1)
src/components/Header/RightSidebar/RightSidebar.tsx (1)
RightSidebar(17-54)
src/app/api/v1/treasury/route.ts (4)
src/utils/getCurrentBlock.ts (1)
getCurrentBlock(8-10)src/app/api/api-utils/getNetworkFromHeaders.ts (1)
getNetworkFromHeaders(10-16)src/global/exceptions.ts (1)
APIError(16-24)src/global/constants/errorCodes.ts (1)
API_ERROR_CODE(5-18)
src/app/page.tsx (2)
src/components/Header/RightSidebar/ActivityFeed.tsx (1)
ActivityFeed(77-187)src/global/types.ts (2)
ActivityFeedItem(660-678)PostFeedListingItem(402-421)
src/components/Header/RightSidebar/RightSidebar.tsx (4)
src/utils/getSubstrateAddress.ts (1)
getSubstrateAddress(13-21)src/components/Header/RightSidebar/QuickActions.tsx (1)
QuickActions(12-55)src/components/Header/RightSidebar/Treasury.tsx (1)
Treasury(31-168)src/components/Header/RightSidebar/ActivityFeed.tsx (1)
ActivityFeed(77-187)
src/app/@modal/(site)/(.)create-proposal/page.tsx (1)
src/contexts/index.tsx (1)
useUserDetailsContext(28-28)
src/components/Misc/EvidenceSelector.tsx (1)
src/utils/getSubstrateAddress.ts (1)
getSubstrateAddress(13-21)
src/components/Header/RightSidebar/Treasury.tsx (2)
src/utils/getSubstrateAddress.ts (1)
getSubstrateAddress(13-21)src/utils/formatSalary.ts (1)
formatSalary(5-20)
🔇 Additional comments (8)
tailwind.config.ts (1)
39-39: Theme color alignment looks good.The primary color change from pink (#E5007A) to blue (#407BFF) aligns with the updated theme in
src/global/themeColors.ts, ensuring consistency across Tailwind tokens and theme color definitions.src/components/Header/Header.module.scss (1)
2-2: Sidebar styling updated for new layout.The styling changes remove rounded corners and adjust padding/borders to accommodate the new RightSidebar component. The use of the new
componentBgtoken ensures theme consistency.src/global/themeColors.ts (2)
7-8: Primary accent color updated across themes.The primary accent color shift from pink (#E5007A) to blue (#407BFF) represents a significant theme change. The primary border adjustment (#D2D8E0 → #C3C8D0) complements the new accent color. These changes align with the corresponding updates in
tailwind.config.ts.
40-41: New theme tokens added for component consistency.The addition of
componentBgandtext_secondarytokens provides better semantic naming for component backgrounds and secondary text, improving theme maintainability.src/app/layout.tsx (2)
10-10: RightSidebar successfully integrated.The new
RightSidebarcomponent is properly integrated with fixed positioning on the right side of the layout. The height adjustment fromh-[96vh]toh-fullon line 28 ensures consistent full-height display for both sidebars.Also applies to: 35-37
41-41: Confirm modal slot integration:{modal}is now rendered withinlayout.tsx; manually verify that the CreateProposal (/@modal/site/create-proposal) and SubmitEvidence (/@modal/site/submit-evidence) modals display as intended.src/global/globals.scss (2)
14-14: Layout updated for full-height sidebar support.The removal of height constraints (
h-[96vh]→h-screen) and width restrictions (w-[90%] max-w-screen-2xl→w-full) enables proper full-viewport display for the new fixed sidebar layout. The removal ofmt-4is compensated by addingpt-4to the main section on line 18.
18-18: Main section margins accommodate both sidebars.The addition of
lg:mr-[332px]provides space for the newRightSidebarcomponent (300px width + borders/padding), whilelg:ml-[296px]accommodates the existingAppSidebar(264px width + padding). Thept-4replaces the removed top margin from the root section.
| <CreateProposalForm | ||
| formRef={formRef} | ||
| onSuccess={() => setIsModalOpen(false)} | ||
| onFormStateChange={(isValid, isLoading) => { | ||
| setIsFormValid(isValid); | ||
| setIsFormLoading(isLoading); | ||
| }} | ||
| /> |
There was a problem hiding this comment.
Route away when the modal succeeds.
Successful proposal creation leaves the modal route in place but with the dialog hidden, so the user hits a blank screen. Mirror the regular close path by sending the router back after setting the modal closed.
- onSuccess={() => setIsModalOpen(false)}
+ onSuccess={() => {
+ setIsModalOpen(false);
+ router.back();
+ }}📝 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.
| <CreateProposalForm | |
| formRef={formRef} | |
| onSuccess={() => setIsModalOpen(false)} | |
| onFormStateChange={(isValid, isLoading) => { | |
| setIsFormValid(isValid); | |
| setIsFormLoading(isLoading); | |
| }} | |
| /> | |
| <CreateProposalForm | |
| formRef={formRef} | |
| onSuccess={() => { | |
| setIsModalOpen(false); | |
| router.back(); | |
| }} | |
| onFormStateChange={(isValid, isLoading) => { | |
| setIsFormValid(isValid); | |
| setIsFormLoading(isLoading); | |
| }} | |
| /> |
🤖 Prompt for AI Agents
In src/app/@modal/(site)/(.)create-proposal/page.tsx around lines 56 to 63, the
onSuccess handler only closes the modal state leaving the modal route in place
which shows a blank page; update the handler to both close the modal AND
navigate the router back to the parent route (mirror the regular close path).
Import and use Next's router (e.g., const router = useRouter() from
'next/navigation') and change onSuccess to call setIsModalOpen(false) followed
by router.back() (or router.push to the appropriate parent path) so the route is
removed after successful creation.
| <SubmitEvidenceForm | ||
| formRef={formRef} | ||
| onSuccess={() => setIsModalOpen(false)} | ||
| onFormStateChange={(isValid, isLoading) => { | ||
| setIsFormValid(isValid); | ||
| setIsFormLoading(isLoading); | ||
| }} | ||
| /> |
There was a problem hiding this comment.
Close modal via router on success.
After a successful submission the modal just hides (isModalOpen = false) while the route remains on /submit-evidence. With nothing else rendered in this interceptor route, users are left staring at a blank screen and must back out manually. Call the same close logic (router.back()) when the form succeeds so the navigation unwinds correctly.
- onSuccess={() => setIsModalOpen(false)}
+ onSuccess={() => {
+ setIsModalOpen(false);
+ router.back();
+ }}📝 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.
| <SubmitEvidenceForm | |
| formRef={formRef} | |
| onSuccess={() => setIsModalOpen(false)} | |
| onFormStateChange={(isValid, isLoading) => { | |
| setIsFormValid(isValid); | |
| setIsFormLoading(isLoading); | |
| }} | |
| /> | |
| <SubmitEvidenceForm | |
| formRef={formRef} | |
| onSuccess={() => { | |
| setIsModalOpen(false); | |
| router.back(); | |
| }} | |
| onFormStateChange={(isValid, isLoading) => { | |
| setIsFormValid(isValid); | |
| setIsFormLoading(isLoading); | |
| }} | |
| /> |
🤖 Prompt for AI Agents
In src/app/@modal/(site)/(.)submit-evidence/page.tsx around lines 56 to 63, the
form success handler only hides the modal by setting isModalOpen=false which
leaves the route at /submit-evidence and results in a blank screen; update the
onSuccess handler to perform the same close navigation used elsewhere (call
router.back() or invoke the existing close function that does router.back())
instead of (or in addition to) just setIsModalOpen(false), and ensure router
(from next/navigation) or the close helper is imported and available in this
component so the route unwinds correctly after successful submission.
| const provider = new WsProvider(wsProvider); | ||
| const api = new ApiPromise({ provider }); | ||
|
|
||
| await api.isReady; | ||
|
|
||
| const treasuryData = await getTreasuryData(api); | ||
|
|
||
| await api.disconnect(); | ||
|
|
||
| return NextResponse.json(treasuryData); | ||
| } catch (error) { | ||
| console.error('Error in treasury API:', error); | ||
| throw new APIError('Failed to fetch treasury data', 500, 'TREASURY_FETCH_ERROR'); | ||
| } |
There was a problem hiding this comment.
Always close the provider on errors.
Line 135 only disconnects the API when everything succeeds; any rejection inside await api.isReady or getTreasuryData leaves the WebSocket open. Under load this leaks connections and exhausts the provider. Wrap the API lifecycle in a try/finally so both success and failure paths call api.disconnect() (and .disconnect() the provider) before bubbling the error.
- const provider = new WsProvider(wsProvider);
- const api = new ApiPromise({ provider });
-
- await api.isReady;
-
- const treasuryData = await getTreasuryData(api);
-
- await api.disconnect();
-
- return NextResponse.json(treasuryData);
+ const provider = new WsProvider(wsProvider);
+ let api: ApiPromise | null = null;
+
+ try {
+ api = new ApiPromise({ provider });
+ await api.isReady;
+
+ const treasuryData = await getTreasuryData(api);
+ return NextResponse.json(treasuryData);
+ } finally {
+ await api?.disconnect();
+ provider.disconnect();
+ }🤖 Prompt for AI Agents
In src/app/api/v1/treasury/route.ts around lines 128 to 141, the ApiPromise and
WsProvider are only disconnected on the success path which leaks WebSocket
connections on errors; wrap the API lifecycle in a try/finally: create the
provider and api, await api.isReady and call getTreasuryData inside a try block,
then in finally await api.disconnect() and call provider.disconnect() (only if
they were created) so both success and error paths close connections before
rethrowing or returning; keep the existing catch that throws the APIError but
ensure disconnect happens in finally.
| const { | ||
| formState: { errors }, | ||
| control, | ||
| handleSubmit | ||
| } = useForm<FormData>({ | ||
| defaultValues: { | ||
| title: '', | ||
| proposalType: ProposalType.PROMOTION, | ||
| motivation: '', | ||
| interest: '', | ||
| evidenceId: searchParams?.get('evidenceId') || '' | ||
| } | ||
| }); | ||
|
|
||
| const [loading, setLoading] = useState(false); | ||
| const [error, setError] = useState(''); | ||
| const [selectedWallet, setSelectedWallet] = useState<Wallet | null>(null); | ||
| const [selectedAddress, setSelectedAddress] = useState<InjectedAccount | null>(null); | ||
| const [txStatus, setTxStatus] = useState(''); | ||
|
|
||
| // Form validation state | ||
| const isFormValid = Boolean(selectedWallet && selectedAddress && api && apiReady && !loading); | ||
|
|
||
| // Notify parent component of form state changes | ||
| React.useEffect(() => { | ||
| onFormStateChange?.(isFormValid, loading); | ||
| }, [isFormValid, loading, onFormStateChange]); | ||
|
|
There was a problem hiding this comment.
Form validity ignores field errors
Line 68 computes isFormValid without considering any react-hook-form validation results, yet Line 72 sends that flag to the parent via onFormStateChange. The parent buttons therefore enable as soon as a wallet/address is picked, even if required fields remain empty, which breaks the Create Proposal flow UX. Please include formState.isValid in this calculation (and enable it via mode: 'onChange' or equivalent) so the parent only treats the form as valid when the inputs actually pass validation. Suggested patch:
-const {
- formState: { errors },
+const {
+ formState: { errors, isValid },
control,
handleSubmit
} = useForm<FormData>({
+ mode: 'onChange',
defaultValues: {
title: '',
proposalType: ProposalType.PROMOTION,
@@
-const isFormValid = Boolean(selectedWallet && selectedAddress && api && apiReady && !loading);
+const isFormValid = Boolean(selectedWallet && selectedAddress && api && apiReady && !loading && isValid);🤖 Prompt for AI Agents
In src/components/CreateProposal/CreateProposalForm.tsx around lines 47 to 74,
isFormValid is computed without considering react-hook-form validation so the
parent is notified the form is valid as soon as a wallet/address is selected;
update useForm to enable validation (e.g., useForm<FormData>({ mode: 'onChange',
defaultValues: { ... } })) and destructure formState.isValid from useForm, then
include that value in the isFormValid boolean (e.g., Boolean(selectedWallet &&
selectedAddress && api && apiReady && !loading && isValid)); finally ensure the
effect that calls onFormStateChange uses the new isValid dependency so the
parent only receives true when form inputs actually pass validation.
| // Sort by created_at date first (most recent first) and limit to 5 items | ||
| const sortedFeedItems = feedItems.toSorted((a, b) => new Date(b.created_at).getTime() - new Date(a.created_at).getTime()).slice(0, 5); | ||
| setFeedItems(sortedFeedItems); |
There was a problem hiding this comment.
toSorted breaks on our runtime
Line 99 calls feedItems.toSorted(...), but Node 18 (our Next.js SSR/runtime target) does not implement the new toSorted helper. This will throw TypeError: feedItems.toSorted is not a function on the server and crash the sidebar. Please fall back to a standard slice().sort(...) instead. Suggested fix:
-const sortedFeedItems = feedItems.toSorted((a, b) => new Date(b.created_at).getTime() - new Date(a.created_at).getTime()).slice(0, 5);
+const sortedFeedItems = feedItems
+ .slice()
+ .sort((a, b) => new Date(b.created_at).getTime() - new Date(a.created_at).getTime())
+ .slice(0, 5);📝 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.
| // Sort by created_at date first (most recent first) and limit to 5 items | |
| const sortedFeedItems = feedItems.toSorted((a, b) => new Date(b.created_at).getTime() - new Date(a.created_at).getTime()).slice(0, 5); | |
| setFeedItems(sortedFeedItems); | |
| // Sort by created_at date first (most recent first) and limit to 5 items | |
| const sortedFeedItems = feedItems | |
| .slice() | |
| .sort((a, b) => new Date(b.created_at).getTime() - new Date(a.created_at).getTime()) | |
| .slice(0, 5); | |
| setFeedItems(sortedFeedItems); |
🤖 Prompt for AI Agents
In src/components/Header/RightSidebar/ActivityFeed.tsx around lines 98 to 100,
replace the use of the unsupported Array.prototype.toSorted with a runtime-safe
approach: copy the array with slice() then call sort(...) with the existing
comparator (new Date(b.created_at).getTime() - new Date(a.created_at).getTime())
to avoid mutating the original, then take the first 5 items with slice(0, 5) and
pass that to setFeedItems; ensure the code uses slice().sort(...).slice(0, 5) so
it works in Node 18/Next.js SSR.
| <Button | ||
| variant='bordered' | ||
| className='font-poppins flex h-16 flex-col gap-1 border-primary_border text-text_secondary hover:bg-gray-50' | ||
| startContent={<DollarSign className='h-4 w-4' />} | ||
| > | ||
| <span className='text-xs'>Claim Salary</span> | ||
| </Button> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Non-functional button without handler.
The "Claim Salary" button has no onClick handler or navigation link, making it non-functional. Consider either implementing the functionality, disabling the button with isDisabled, or adding an onClick handler that shows a "Coming Soon" notification.
Apply this diff if the feature is not yet implemented:
<Button
variant='bordered'
className='font-poppins flex h-16 flex-col gap-1 border-primary_border text-text_secondary hover:bg-gray-50'
startContent={<DollarSign className='h-4 w-4' />}
+ isDisabled
+ title='Coming soon'
>
<span className='text-xs'>Claim Salary</span>
</Button>🏁 Script executed:
#!/bin/bash
# Show lines around the "Claim Salary" button
rg -n 'Claim Salary' -C5 src/components/Header/RightSidebar/QuickActions.tsx
# Search for any onClick handlers in the file
rg -n 'onClick' -C3 src/components/Header/RightSidebar/QuickActions.tsxLength of output: 459
Disable or implement the “Claim Salary” button (QuickActions.tsx:27-33)
The <Button> has no onClick or navigation – add an onClick handler or, if unimplemented, disable it (isDisabled title="Coming soon").
<Button
variant='bordered'
className='font-poppins flex h-16 flex-col gap-1 border-primary_border text-text_secondary hover:bg-gray-50'
startContent={<DollarSign className='h-4 w-4' />}
+ isDisabled
+ title='Coming soon'
>
<span className='text-xs'>Claim Salary</span>
</Button>📝 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.
| <Button | |
| variant='bordered' | |
| className='font-poppins flex h-16 flex-col gap-1 border-primary_border text-text_secondary hover:bg-gray-50' | |
| startContent={<DollarSign className='h-4 w-4' />} | |
| > | |
| <span className='text-xs'>Claim Salary</span> | |
| </Button> | |
| <Button | |
| variant='bordered' | |
| className='font-poppins flex h-16 flex-col gap-1 border-primary_border text-text_secondary hover:bg-gray-50' | |
| startContent={<DollarSign className='h-4 w-4' />} | |
| isDisabled | |
| title='Coming soon' | |
| > | |
| <span className='text-xs'>Claim Salary</span> | |
| </Button> |
🤖 Prompt for AI Agents
In src/components/Header/RightSidebar/QuickActions.tsx around lines 27 to 33 the
"Claim Salary" <Button> lacks an onClick handler or a disabled state; either
implement its behavior (add a clear onClick that performs the action or routes
to the appropriate page and handle success/error) or mark it as disabled with
isDisabled and a title like "Coming soon" to communicate that it's
unimplemented; ensure accessible attributes (aria-disabled/title) are set when
disabling.
| disabled={disabled} | ||
| onWalletClick={(e: React.MouseEvent, wallet: Wallet) => setTempSelectedWallet(wallet)} | ||
| /> | ||
| </div> | ||
|
|
||
| {tempSelectedWallet && ( | ||
| <div> | ||
| <h4 className='mb-3 text-sm font-medium'>Select Address</h4> | ||
| <AddressDropdown | ||
| wallet={tempSelectedWallet} | ||
| onAddressSelect={setTempSelectedAddress} | ||
| disabled={disabled} | ||
| /> |
There was a problem hiding this comment.
Switching wallets keeps the old address
Line 166 updates tempSelectedWallet, but tempSelectedAddress is left untouched, so the previous wallet’s address stays truthy. Because canConfirm (Line 101) only checks for truthiness, users can confirm a new wallet while still submitting the old address, creating an inconsistent wallet/address pair. Clear the temp address when the wallet changes so the user must pick a matching address:
<WalletButtonsRow
disabled={disabled}
- onWalletClick={(e: React.MouseEvent, wallet: Wallet) => setTempSelectedWallet(wallet)}
+ onWalletClick={(e: React.MouseEvent, wallet: Wallet) => {
+ setTempSelectedWallet(wallet);
+ setTempSelectedAddress(null);
+ }}
/>🤖 Prompt for AI Agents
In src/components/Misc/AddressSwitch.tsx around lines 165 to 177, updating
tempSelectedWallet without clearing tempSelectedAddress leaves the previous
wallet’s address truthy and allows canConfirm (checked at line 101) to pass with
a mismatched wallet/address pair; update the wallet click handler so that when
setting tempSelectedWallet it also clears tempSelectedAddress (set it to
undefined/null) so the user must select an address that matches the newly chosen
wallet.
| const onSuccessCallback = async () => { | ||
| setLoading(false); | ||
| // Generate a mock evidence ID - in real implementation, this would come from the transaction | ||
| const evidenceId = `evidence_${Date.now()}`; | ||
| setSubmittedEvidenceId(evidenceId); | ||
| setIsSuccess(true); | ||
|
|
||
| queueNotification({ | ||
| header: 'Evidence Submitted Successfully!', | ||
| message: `Your ${wish.toLowerCase()} evidence has been submitted for ${fellow.rank} rank.`, | ||
| status: 'success' | ||
| }); | ||
| setTxStatus(''); | ||
| }; |
There was a problem hiding this comment.
Trigger the provided success callback.
The component accepts an onSuccess prop, but Line 185 never actually invokes it when the transaction succeeds. As a result, the page/modal wrappers can’t react (e.g., closing the modal or refreshing data). Call onSuccess?.() inside onSuccessCallback after updating local state.
const onSuccessCallback = async () => {
setLoading(false);
// Generate a mock evidence ID - in real implementation, this would come from the transaction
const evidenceId = `evidence_${Date.now()}`;
setSubmittedEvidenceId(evidenceId);
setIsSuccess(true);
queueNotification({
header: 'Evidence Submitted Successfully!',
message: `Your ${wish.toLowerCase()} evidence has been submitted for ${fellow.rank} rank.`,
status: 'success'
});
setTxStatus('');
+ onSuccess?.();
};📝 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.
| const onSuccessCallback = async () => { | |
| setLoading(false); | |
| // Generate a mock evidence ID - in real implementation, this would come from the transaction | |
| const evidenceId = `evidence_${Date.now()}`; | |
| setSubmittedEvidenceId(evidenceId); | |
| setIsSuccess(true); | |
| queueNotification({ | |
| header: 'Evidence Submitted Successfully!', | |
| message: `Your ${wish.toLowerCase()} evidence has been submitted for ${fellow.rank} rank.`, | |
| status: 'success' | |
| }); | |
| setTxStatus(''); | |
| }; | |
| const onSuccessCallback = async () => { | |
| setLoading(false); | |
| // Generate a mock evidence ID - in real implementation, this would come from the transaction | |
| const evidenceId = `evidence_${Date.now()}`; | |
| setSubmittedEvidenceId(evidenceId); | |
| setIsSuccess(true); | |
| queueNotification({ | |
| header: 'Evidence Submitted Successfully!', | |
| message: `Your ${wish.toLowerCase()} evidence has been submitted for ${fellow.rank} rank.`, | |
| status: 'success' | |
| }); | |
| setTxStatus(''); | |
| onSuccess?.(); | |
| }; |
🤖 Prompt for AI Agents
In src/components/SubmitEvidence/SubmitEvidenceForm.tsx around lines 184 to 197,
the onSuccess prop is never invoked in onSuccessCallback so parent wrappers
can't react; after setting local state (setLoading, setSubmittedEvidenceId,
setIsSuccess) and before or after queueNotification/setTxStatus, call
onSuccess?.() to trigger the provided success handler (use optional chaining to
avoid errors if prop is undefined).
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/utils/getAllFellowAddresses.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/utils/getAllFellowAddresses.ts (1)
src/utils/getSubstrateAddress.ts (1)
getSubstrateAddress(13-21)
src/utils/getAllFellowAddresses.ts
Outdated
| // Whitelisted fellows (for testing or manual additions) | ||
| const WHITELISTED_FELLOWS: string[] = ['YS7UCpmCREYUa3dgbVWfQQT88Vi9MZGiyxc5DRoaEysvFSz']; |
There was a problem hiding this comment.
Move test addresses to configuration or remove from production code.
Hardcoding test addresses in production source code is problematic:
- Configuration: This should be externalized to environment variables or a config file
- Testing: If this is for testing purposes, it should be behind a feature flag or removed before deployment
- Clarity: The comment "for testing or manual additions" is vague about intent and lifecycle
Consider one of these approaches:
-// Whitelisted fellows (for testing or manual additions)
-const WHITELISTED_FELLOWS: string[] = ['YS7UCpmCREYUa3dgbVWfQQT88Vi9MZGiyxc5DRoaEysvFSz'];
+// Load whitelisted fellows from environment if present
+const WHITELISTED_FELLOWS: string[] = process.env.NEXT_PUBLIC_WHITELISTED_FELLOWS?.split(',').filter(Boolean) || [];Or remove entirely if this is temporary testing code.
🤖 Prompt for AI Agents
In src/utils/getAllFellowAddresses.ts around lines 11-12, a test address is
hardcoded in WHITELISTED_FELLOWS which should not live in production source;
remove the hardcoded value and either (a) load whitelist from configuration/env
(e.g. parse a CSV/JSON string from process.env.WHITELISTED_FELLOWS or a config
file) with a safe default of an empty array, or (b) gate inclusion behind a
clearly named feature flag so test addresses are only used in non-production
environments; ensure the code validates/parses the config value into a string[]
and document the env/config key.
| // Add whitelisted fellows if not already in the list | ||
| WHITELISTED_FELLOWS.forEach((whitelistedAddress) => { | ||
| const substrateAddress = getSubstrateAddress(whitelistedAddress) || whitelistedAddress; | ||
| const alreadyExists = members.some((m) => m.address === substrateAddress); | ||
|
|
||
| if (!alreadyExists) { | ||
| // Add with rank 1 (Member) and default params | ||
| members.push({ | ||
| address: substrateAddress, | ||
| rank: 1, | ||
| salary: activeSalary?.[1] || 0, | ||
| params: { | ||
| activeSalary: activeSalary?.[1] || 0, | ||
| demotionPeriod: demotionPeriod?.[1] || 0, | ||
| minPromotionPeriod: minPromotionPeriod?.[1] || 0, | ||
| offboardTimeout, | ||
| passiveSalary: passiveSalary?.[1] || 0 | ||
| } | ||
| }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Bypassing on-chain membership validation creates security and data integrity risks.
Augmenting the on-chain fellowship data with hardcoded addresses introduces several concerns:
- Authorization: These addresses gain fellow status without on-chain membership, potentially accessing restricted features
- Data integrity: The UI displays members that don't exist on-chain, creating a discrepancy between UI state and blockchain truth
- Misleading information: Users may make decisions based on incorrect fellowship composition and member counts
This pattern should only be used in development/staging environments with clear safeguards.
Consider wrapping this logic in a development-only check:
- // Add whitelisted fellows if not already in the list
- WHITELISTED_FELLOWS.forEach((whitelistedAddress) => {
+ // Add whitelisted fellows if not already in the list (dev/staging only)
+ if (process.env.NODE_ENV !== 'production' && WHITELISTED_FELLOWS.length > 0) {
+ WHITELISTED_FELLOWS.forEach((whitelistedAddress) => {
- const substrateAddress = getSubstrateAddress(whitelistedAddress) || whitelistedAddress;
- const alreadyExists = members.some((m) => m.address === substrateAddress);
-
- if (!alreadyExists) {
- // Add with rank 1 (Member) and default params
- members.push({
- address: substrateAddress,
- rank: 1,
- salary: activeSalary?.[1] || 0,
- params: {
- activeSalary: activeSalary?.[1] || 0,
- demotionPeriod: demotionPeriod?.[1] || 0,
- minPromotionPeriod: minPromotionPeriod?.[1] || 0,
- offboardTimeout,
- passiveSalary: passiveSalary?.[1] || 0
- }
- });
- }
- });
+ const substrateAddress = getSubstrateAddress(whitelistedAddress) || whitelistedAddress;
+ const alreadyExists = members.some((m) => m.address === substrateAddress);
+
+ if (!alreadyExists) {
+ // Add with rank 1 (Member) and default params
+ members.push({
+ address: substrateAddress,
+ rank: 1,
+ salary: activeSalary?.[1] || 0,
+ params: {
+ activeSalary: activeSalary?.[1] || 0,
+ demotionPeriod: demotionPeriod?.[1] || 0,
+ minPromotionPeriod: minPromotionPeriod?.[1] || 0,
+ offboardTimeout,
+ passiveSalary: passiveSalary?.[1] || 0
+ }
+ });
+ }
+ });
+ }📝 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.
| // Add whitelisted fellows if not already in the list | |
| WHITELISTED_FELLOWS.forEach((whitelistedAddress) => { | |
| const substrateAddress = getSubstrateAddress(whitelistedAddress) || whitelistedAddress; | |
| const alreadyExists = members.some((m) => m.address === substrateAddress); | |
| if (!alreadyExists) { | |
| // Add with rank 1 (Member) and default params | |
| members.push({ | |
| address: substrateAddress, | |
| rank: 1, | |
| salary: activeSalary?.[1] || 0, | |
| params: { | |
| activeSalary: activeSalary?.[1] || 0, | |
| demotionPeriod: demotionPeriod?.[1] || 0, | |
| minPromotionPeriod: minPromotionPeriod?.[1] || 0, | |
| offboardTimeout, | |
| passiveSalary: passiveSalary?.[1] || 0 | |
| } | |
| }); | |
| } | |
| }); | |
| // Add whitelisted fellows if not already in the list (dev/staging only) | |
| if (process.env.NODE_ENV !== 'production' && WHITELISTED_FELLOWS.length > 0) { | |
| WHITELISTED_FELLOWS.forEach((whitelistedAddress) => { | |
| const substrateAddress = getSubstrateAddress(whitelistedAddress) || whitelistedAddress; | |
| const alreadyExists = members.some((m) => m.address === substrateAddress); | |
| if (!alreadyExists) { | |
| // Add with rank 1 (Member) and default params | |
| members.push({ | |
| address: substrateAddress, | |
| rank: 1, | |
| salary: activeSalary?.[1] || 0, | |
| params: { | |
| activeSalary: activeSalary?.[1] || 0, | |
| demotionPeriod: demotionPeriod?.[1] || 0, | |
| minPromotionPeriod: minPromotionPeriod?.[1] || 0, | |
| offboardTimeout, | |
| passiveSalary: passiveSalary?.[1] || 0 | |
| } | |
| }); | |
| } | |
| }); | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (1)
src/components/Header/RightSidebar/ActivityFeed.tsx (1)
108-111:toSortedis unsupported in Node 18 runtime (duplicate).Line 110 uses
Array.prototype.toSorted, which is not available in Node 18 and will cause a runtime error in Next.js SSR. This issue was already flagged in a previous review.As previously noted, apply this fix:
-const sortedFeedItems = feedItems.toSorted((a, b) => new Date(b.created_at).getTime() - new Date(a.created_at).getTime()).slice(0, 5); +const sortedFeedItems = feedItems + .slice() + .sort((a, b) => new Date(b.created_at).getTime() - new Date(a.created_at).getTime()) + .slice(0, 5);
🧹 Nitpick comments (8)
src/components/Header/AppSidebar.tsx (2)
13-13: Clarify commented-out code or remove it.Lines 13 and 143 contain commented-out references to
JoinFellowshipButton. If this component is being removed or temporarily disabled, consider either:
- Removing the commented code if it's no longer needed.
- Adding a TODO comment explaining why it's disabled and when it should be restored.
Leaving commented-out code without explanation can create confusion for future maintainers.
Also applies to: 143-143
173-189: Consider extracting className logic for readability.Lines 175-179 contain complex conditional className logic that could be refactored for better readability. While functional, extracting this into a helper function or using a classnames utility would improve maintainability.
Consider this refactor:
+const getListboxItemClassName = (isCurrentRoute: boolean, isParentItem: boolean) => { + const base = 'mb-3 rounded p-2 transition-colors'; + if (isCurrentRoute && !isParentItem) { + return `${base} border-l-4 border-primary_accent bg-primary_accent/10 font-semibold text-primary_accent`; + } + const parentCurrent = isParentItem && isCurrentRoute ? 'text-primary_accent' : ''; + return `${base} hover:text-text_primary text-text_secondary hover:bg-gray-50 ${parentCurrent}`; +}; <ListboxItem id='nav-listbox-item' - className={`mb-3 rounded p-2 transition-colors ${ - isCurrentRoute && !isParentItem - ? 'border-l-4 border-primary_accent bg-primary_accent/10 font-semibold text-primary_accent' - : 'hover:text-text_primary text-text_secondary hover:bg-gray-50' - } ${isParentItem && isCurrentRoute && 'text-primary_accent'}`} + className={getListboxItemClassName(isCurrentRoute, isParentItem)} key={navItem.url}src/components/Profile/RequiredActions.tsx (1)
39-96: Consider making action item counts dynamic.The component displays hardcoded counts (2 pending proposals, 4 junior members) that should ideally come from props or API data to reflect actual state.
Consider updating the Props interface and component to accept dynamic data:
interface Props { className?: string; + pendingProposalsCount?: number; + membersToGradeCount?: number; + requiresEvidence?: boolean; }Then use these props to conditionally render sections and display actual counts.
src/components/Profile/RecentContributions.tsx (1)
68-72: Guard against null reference when accessing githubStats.Line 71 accesses
githubStats?.isIncreasewhich could be undefined during initial render before the fetch completes, though the optional chaining prevents crashes.Consider making the fallback logic more explicit:
const contributionStats: ContributionStats = { githubCommits: { count: githubStats?.totalContributionsCount || 47, - change: githubStats?.isIncrease ? 12 : -5, + change: githubStats ? (githubStats.isIncrease ? 12 : -5) : 12, period: 'This month' },src/components/Profile/index.tsx (1)
43-58: Remove or convert large commented code blocks.Large blocks of commented-out code should either be removed (if no longer needed) or converted to TODO comments with context about future plans.
If these components (ContributionGraph, Manifesto, ProfileProposals) will be reintroduced later:
- {/* <ContributionGraph - classNames='mt-[56px]' - githubUsername={socialLinks?.find((social) => social.type === 'Github')?.link || ''} - openProfileEdit={openProfileEdit} - /> */} - {/* <div className='mt-4 flex flex-col gap-4 md:grid md:grid-cols-11'> - <section className='flex w-full flex-col gap-4 md:col-span-5'> - <Manifesto - manifesto={manifesto} - address={address} - /> - </section> - <section className='w-full md:col-span-6'> - <ProfileProposals address={address} /> - </section> - </div> */} + {/* TODO: Re-enable ContributionGraph, Manifesto, and ProfileProposals sections after completing the V2 UI migration */}Otherwise, remove the commented code entirely.
src/components/Profile/DashboardHeader.tsx (1)
45-50: Add null safety to address comparison logic.The comparison on Line 49 could fail if
getSubstrateAddressreturnsnullfor invalid addresses, though the current logic would safely returnfalsein that case.Consider making the null handling more explicit:
const isLoggedInUserProfile = useMemo(() => { const substrateAddress = getSubstrateAddress(address); + const loginSubstrateAddress = getSubstrateAddress(loginAddress); + return substrateAddress !== null && loginSubstrateAddress !== null && loginSubstrateAddress === substrateAddress; - return getSubstrateAddress(loginAddress) === substrateAddress; }, [address, loginAddress]);src/components/Profile/ProfileCards.tsx (2)
46-48: Add null safety for rank access.Line 47 accesses
fellow.rankwithout verifying thatfellowexists, relying only on the ternary at the start of the expression.Consider making the logic more defensive:
<div className='flex justify-between text-sm'> - <span className='text-text_secondary'>{fellow ? `Dan ${fellow.rank} → Dan ${fellow.rank + 1}` : 'Member → Fellow'}</span> + <span className='text-text_secondary'>{fellow?.rank ? `Dan ${fellow.rank} → Dan ${fellow.rank + 1}` : 'Member → Fellow'}</span> <span className='font-semibold text-black dark:text-white'>{promotionProgress}%</span> </div>
94-94: Note: currentSalary type inconsistency.The
currentSalaryis extracted as a string (Line 27:|| '0') but displayed as if it's a numeric value. While this works for display purposes, it may cause issues if arithmetic operations are needed later.Consider parsing the salary as a number if needed for calculations:
- const currentSalary = fellow?.params?.activeSalary?.[0] || '0'; + const currentSalary = Number(fellow?.params?.activeSalary?.[0]) || 0;Then update the display:
- <div className='text-2xl font-semibold text-black dark:text-white'>{currentSalary} DOT</div> + <div className='text-2xl font-semibold text-black dark:text-white'>{currentSalary.toLocaleString()} DOT</div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/app/page.tsx(2 hunks)src/components/Header/AppSidebar.tsx(6 hunks)src/components/Header/RightSidebar/ActivityFeed.tsx(1 hunks)src/components/Header/RightSidebar/RightSidebar.tsx(1 hunks)src/components/Home/PendingTasks.tsx(0 hunks)src/components/Profile/DashboardHeader.tsx(1 hunks)src/components/Profile/ProfileCards.tsx(1 hunks)src/components/Profile/RecentContributions.tsx(1 hunks)src/components/Profile/RequiredActions.tsx(1 hunks)src/components/Profile/Socials/SocialIcon.tsx(1 hunks)src/components/Profile/Socials/index.tsx(1 hunks)src/components/Profile/index.tsx(1 hunks)src/utils/getAllFellowAddresses.ts(2 hunks)tailwind.config.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/components/Home/PendingTasks.tsx
✅ Files skipped from review due to trivial changes (1)
- src/components/Profile/Socials/index.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/utils/getAllFellowAddresses.ts
- tailwind.config.ts
🧰 Additional context used
🧬 Code graph analysis (7)
src/app/page.tsx (1)
src/global/types.ts (1)
ServerComponentProps(91-94)
src/components/Profile/ProfileCards.tsx (1)
src/contexts/index.tsx (1)
useApiContext(28-28)
src/components/Header/RightSidebar/ActivityFeed.tsx (4)
src/global/types.ts (1)
ActivityFeedItem(660-678)src/utils/getSubstrateAddress.ts (1)
getSubstrateAddress(13-21)src/utils/getOriginUrl.ts (1)
getOriginUrl(7-12)src/app/api/v1/feed/getActivityFeed.ts (1)
getActivityFeed(20-34)
src/components/Profile/index.tsx (1)
src/global/types.ts (1)
IProfile(650-656)
src/components/Profile/RecentContributions.tsx (1)
src/utils/getGithubMonthlyStats.ts (1)
getGithubMonthlyStats(8-44)
src/components/Profile/DashboardHeader.tsx (5)
src/global/types.ts (1)
ISocial(645-648)src/hooks/useIdentity.ts (1)
useIdentity(9-40)src/utils/getEncodedAddress.ts (1)
getEncodedAddress(15-29)src/utils/getSubstrateAddress.ts (1)
getSubstrateAddress(13-21)src/utils/midTruncateText.ts (1)
midTruncateText(20-24)
src/components/Header/RightSidebar/RightSidebar.tsx (5)
src/contexts/index.tsx (2)
useApiContext(28-28)useUserDetailsContext(28-28)src/utils/getSubstrateAddress.ts (1)
getSubstrateAddress(13-21)src/components/Header/RightSidebar/QuickActions.tsx (1)
QuickActions(12-55)src/components/Header/RightSidebar/Treasury.tsx (1)
Treasury(31-168)src/components/Header/RightSidebar/ActivityFeed.tsx (1)
ActivityFeed(88-198)
🔇 Additional comments (11)
src/app/page.tsx (2)
29-29: LGTM: Readonly wrapper follows Next.js best practices.Wrapping
ServerComponentPropswithReadonly<>ensuressearchParamsimmutability in this Server Component, aligning with Next.js 13+ recommendations.
47-47: Layout simplification aligns with V2 UI refactor.Removing the width constraint allows the left column to expand naturally now that the right sidebar has been moved to the root layout. This change is consistent with the broader V2 UI restructuring.
src/components/Header/RightSidebar/RightSidebar.tsx (2)
29-58: LGTM! Clean conditional UI based on fellowship status.The header section appropriately renders different actions based on whether the user is a fellow:
- Non-fellows see a "Join Fellowship" call-to-action.
- Fellows see a "Create Rank Request" action.
The implementation uses proper navigation components and follows consistent styling patterns.
59-66: Add loading state indicators for fellows data in context, or exposefellowsLoadingflag.The
useApiContext()currently provides no loading or error state for thefellowsdata fetched asynchronously (line 99 ofApiContext.tsx). While child components likeRightSidebar,Treasury, andActivityFeeduse optional chaining to handle undefined values, they cannot distinguish between "still loading" and "fetch completed with no data". This may cause layout shifts or missing content without user feedback during initial load or when network is slow.Either:
- Add
fellowsLoading: booleanandfellowsError: string | nullto theApiContextTypeand expose them from the provider, or- Implement local loading states in
TreasuryandActivityFeedif they independently fetch or depend on fellows data.src/components/Header/AppSidebar.tsx (3)
20-33: LGTM! Proper icon component integration.The
ListboxItemStartContentcomponent correctly handles the migration from string icon identifiers to React icon components. The typing is accurate (React.ComponentType<{ className?: string }>), and the conditional rendering and styling are appropriate.
35-105: LGTM! Complete migration to icon components.The
navItemsarray has been successfully updated to use React icon components from lucide-react instead of string identifiers. The type definition correctly reflectsicon?: React.ComponentType<{ className?: string }>, and all entries use the appropriate imported icon components (Home, Vote, Users, etc.).This improves type safety and enables better tree-shaking since only used icons are bundled.
116-142: LGTM! Improved branding and header layout.The new header section effectively presents the Collectives branding and clearly attributes governance to Polkassembly. The layout is clean and the visual hierarchy is appropriate.
src/components/Header/RightSidebar/ActivityFeed.tsx (1)
184-192: Keep href="/" for “View All Activity”
The root path loads the Home page, which defaults to the full activity feed.src/components/Profile/Socials/SocialIcon.tsx (1)
14-14: LGTM! Size reduction aligns with the profile UI updates.The container size reduction from
h-10 w-10toh-8 w-8is a minor visual adjustment that maintains consistency with the broader profile UI refinements described in this PR.src/components/Profile/index.tsx (1)
30-42: LGTM! Profile layout restructuring looks clean.The new component composition with DashboardHeader, RequiredActions, ProfileCards, and RecentContributions provides a clearer, more modular structure compared to the previous layout.
src/components/Profile/DashboardHeader.tsx (1)
62-64: Note: Status toggle is UI-only without backend integration.The
handleStatusToggleupdates local state but doesn't persist changes to any backend or API, so the status will reset on page reload.Ensure that the status toggle is intended to be UI-only for this phase, or add backend integration to persist the status change.
| // Simple function to get activity details | ||
| const getActivityDetails = (feedItem: ActivityFeedItem, fellows: any[] = []) => { | ||
| switch (feedItem.type) { | ||
| case SubsquidActivityType.Inducted: | ||
| return { icon: UserPlus, color: 'text-green-600', title: 'Member Inducted', description: 'was inducted into the fellowship' }; | ||
| case SubsquidActivityType.Promoted: | ||
| return { icon: TrendingUp, color: 'text-green-600', title: 'Member Promoted', description: `was promoted to Rank ${feedItem.rank || 0}` }; | ||
| case SubsquidActivityType.Demoted: | ||
| return { icon: TrendingDown, color: 'text-red-600', title: 'Member Demoted', description: `was demoted to Rank ${feedItem.rank || 0}` }; | ||
| case SubsquidActivityType.Retained: { | ||
| // For retained activities, if rank is 0, try to get the current rank from fellows data | ||
| let retainedRank = feedItem.rank || 0; | ||
| if (retainedRank === 0) { | ||
| const substrateAddress = getSubstrateAddress(feedItem.who); | ||
| const fellow = fellows.find((f) => f.address === substrateAddress); | ||
| if (fellow) { | ||
| retainedRank = fellow.rank; | ||
| } | ||
| } | ||
| return { icon: CircleCheckBig, color: 'text-green-600', title: 'Member Retained', description: `was retained at Rank ${retainedRank}` }; | ||
| } | ||
| case SubsquidActivityType.EvidenceSubmitted: | ||
| return { icon: FileText, color: 'text-purple-600', title: 'Evidence Submitted', description: 'submitted new evidence' }; | ||
| case SubsquidActivityType.EvidenceJudged: | ||
| return { icon: FileText, color: 'text-indigo-600', title: 'Evidence Judged', description: 'had their evidence judged' }; | ||
| case SubsquidActivityType.GeneralProposal: | ||
| case SubsquidActivityType.RFC: | ||
| return { icon: Vote, color: 'text-blue-600', title: 'Proposal Created', description: 'created a new proposal' }; | ||
| case SubsquidActivityType.PromotionRequest: | ||
| return { icon: TrendingUp, color: 'text-blue-600', title: 'Promotion Request', description: 'requested promotion' }; | ||
| case SubsquidActivityType.RetentionRequest: | ||
| return { icon: CircleCheckBig, color: 'text-blue-600', title: 'Retention Request', description: 'requested retention' }; | ||
| case SubsquidActivityType.DemotionRequest: | ||
| return { icon: TrendingDown, color: 'text-orange-600', title: 'Demotion Request', description: 'requested demotion' }; | ||
| case SubsquidActivityType.InductionRequest: | ||
| return { icon: UserPlus, color: 'text-blue-600', title: 'Induction Request', description: 'requested induction' }; | ||
| case SubsquidActivityType.Registration: | ||
| return { | ||
| icon: DollarSign, | ||
| color: 'text-green-600', | ||
| title: 'Salary Registration', | ||
| description: `registered for salary on ${feedItem.cycleStartDatetime ? dayjs(feedItem.cycleStartDatetime).format('DD MMM YYYY') : ''}`.trim() | ||
| }; | ||
| case SubsquidActivityType.SalaryInduction: | ||
| return { icon: DollarSign, color: 'text-green-600', title: 'Salary Induction', description: 'was inducted into salary system' }; | ||
| case SubsquidActivityType.Payout: | ||
| return { icon: DollarSign, color: 'text-yellow-600', title: 'Salary Payout', description: 'received salary payout' }; | ||
| case SubsquidActivityType.CycleStarted: | ||
| return { icon: Settings, color: 'text-blue-600', title: 'Cycle Started', description: 'salary cycle started' }; | ||
| case SubsquidActivityType.Voted: | ||
| return { | ||
| icon: Vote, | ||
| color: 'text-blue-600', | ||
| title: 'Vote Cast', | ||
| description: `voted ${feedItem.vote?.decision || 'unknown'} on proposal #${feedItem.vote?.proposalIndex || 'unknown'}` | ||
| }; | ||
| case SubsquidActivityType.OffBoarded: | ||
| return { icon: TrendingDown, color: 'text-red-600', title: 'Member Off-boarded', description: 'was off-boarded from the fellowship' }; | ||
| case SubsquidActivityType.ActivityChanged: | ||
| return { icon: Settings, color: 'text-gray-600', title: 'Status Changed', description: `status changed to ${feedItem.isActive ? 'active' : 'inactive'}` }; | ||
| case SubsquidActivityType.Imported: | ||
| return { icon: Settings, color: 'text-gray-600', title: 'Member Imported', description: 'was imported into the system' }; | ||
| default: | ||
| return { icon: CircleCheckBig, color: 'text-blue-600', title: 'Activity', description: 'performed an activity' }; | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace any[] type with proper Fellow type.
Line 22 declares the fellows parameter as any[], which bypasses TypeScript's type checking. Use a proper Fellow type (likely available in your global types) to enable autocomplete, catch errors at compile-time, and improve maintainability.
Apply this diff:
-const getActivityDetails = (feedItem: ActivityFeedItem, fellows: any[] = []) => {
+const getActivityDetails = (feedItem: ActivityFeedItem, fellows: Fellow[] = []) => {Ensure Fellow is imported from your types file (e.g., import { Fellow } from '@/global/types').
🤖 Prompt for AI Agents
In src/components/Header/RightSidebar/ActivityFeed.tsx around lines 21 to 86,
the getActivityDetails function currently types the fellows parameter as any[]
which defeats TypeScript checks; replace any[] with the proper Fellow[] type and
add an import for Fellow from your types file (for example import { Fellow }
from '@/global/types'), then update the function signature to use fellows:
Fellow[] = [] so you get compile-time checking and editor autocomplete.
| const isFellow = useMemo(() => { | ||
| if (!loginAddress || !fellows?.length) return false; | ||
| const substrateAddress = getSubstrateAddress(loginAddress); | ||
| return fellows.find((f: any) => f.address === substrateAddress) !== undefined; | ||
| }, [loginAddress, fellows]); |
There was a problem hiding this comment.
Improve type safety and handle null address.
The isFellow computation has type safety and correctness issues:
- Line 26 uses
anytype, bypassing TypeScript checks. Use a properFellowtype from your types file. getSubstrateAddresscan returnnull, but there's no null-check before comparing addresses, potentially causing false negatives.
Apply this diff to improve type safety:
const isFellow = useMemo(() => {
if (!loginAddress || !fellows?.length) return false;
const substrateAddress = getSubstrateAddress(loginAddress);
- return fellows.find((f: any) => f.address === substrateAddress) !== undefined;
+ if (!substrateAddress) return false;
+ return fellows.find((f) => f.address === substrateAddress) !== undefined;
}, [loginAddress, fellows]);Note: Ensure the fellows array from useApiContext is properly typed. If not, add a type annotation like Fellow[] where Fellow is imported from your types file.
📝 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.
| const isFellow = useMemo(() => { | |
| if (!loginAddress || !fellows?.length) return false; | |
| const substrateAddress = getSubstrateAddress(loginAddress); | |
| return fellows.find((f: any) => f.address === substrateAddress) !== undefined; | |
| }, [loginAddress, fellows]); | |
| const isFellow = useMemo(() => { | |
| if (!loginAddress || !fellows?.length) return false; | |
| const substrateAddress = getSubstrateAddress(loginAddress); | |
| if (!substrateAddress) return false; | |
| return fellows.find((f) => f.address === substrateAddress) !== undefined; | |
| }, [loginAddress, fellows]); |
🤖 Prompt for AI Agents
In src/components/Header/RightSidebar/RightSidebar.tsx around lines 23 to 27,
the isFellow useMemo uses any and doesn't handle a null substrate address;
update it to use the proper Fellow type (import Fellow from your types file and
ensure the fellows source is typed as Fellow[]), replace the (f: any) signature
with (f: Fellow), and call getSubstrateAddress(loginAddress) into a local const
and return false immediately if that call returns null before doing the
fellows.find address comparison so the comparison is type-safe and correct.
| console.log('substrateAddress', substrateAddress); | ||
| console.log('loginAddress', loginAddress); |
There was a problem hiding this comment.
Remove debug console.log statements.
Debug logging statements should not be present in production code.
Apply this diff to remove the console logs:
const isLoggedInUserProfile = useMemo(() => {
const substrateAddress = getSubstrateAddress(address);
- console.log('substrateAddress', substrateAddress);
- console.log('loginAddress', loginAddress);
return getSubstrateAddress(loginAddress) === substrateAddress;
}, [address, loginAddress]);📝 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.
| console.log('substrateAddress', substrateAddress); | |
| console.log('loginAddress', loginAddress); | |
| const isLoggedInUserProfile = useMemo(() => { | |
| const substrateAddress = getSubstrateAddress(address); | |
| return getSubstrateAddress(loginAddress) === substrateAddress; | |
| }, [address, loginAddress]); |
🤖 Prompt for AI Agents
In src/components/Profile/DashboardHeader.tsx around lines 47 to 48, remove the
two debug console.log statements that print substrateAddress and loginAddress;
delete those lines (or replace with a proper logging call if required by project
logging standards) so no console.debug/console.log calls remain in production
code.
| return getSubstrateAddress(loginAddress) === substrateAddress; | ||
| }, [address, loginAddress]); | ||
|
|
||
| console.log('isLoggedInUserProfile', isLoggedInUserProfile); |
There was a problem hiding this comment.
Remove debug console.log statement.
Another debug logging statement that should not be in production code.
Apply this diff:
- console.log('isLoggedInUserProfile', isLoggedInUserProfile);
-📝 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.
| console.log('isLoggedInUserProfile', isLoggedInUserProfile); |
🤖 Prompt for AI Agents
In src/components/Profile/DashboardHeader.tsx around line 52 there is a debug
console.log call (console.log('isLoggedInUserProfile', isLoggedInUserProfile));
remove this statement from the file so no debug logging remains in production;
if the value is needed for future debugging, replace with a commented note or
use a proper logger behind a debug flag instead.
| <p className='text-sm leading-relaxed text-secondaryText'> | ||
| Experienced blockchain researcher specializing in consensus mechanisms and cryptographic protocols. Active contributor to Polkadot ecosystem with focus on runtime | ||
| optimization and security research. Published author and speaker at major blockchain conferences. | ||
| </p> |
There was a problem hiding this comment.
Replace hardcoded biography with dynamic content.
The biography section contains placeholder text that should be replaced with actual profile data.
Consider fetching biography from the profile data or identity:
<div className='mt-4 border-t border-primary_border pt-4'>
- <p className='text-sm leading-relaxed text-secondaryText'>
- Experienced blockchain researcher specializing in consensus mechanisms and cryptographic protocols. Active contributor to Polkadot ecosystem with focus on runtime
- optimization and security research. Published author and speaker at major blockchain conferences.
- </p>
+ {onChainIdentity?.web && (
+ <p className='text-sm leading-relaxed text-secondaryText'>
+ {onChainIdentity.web}
+ </p>
+ )}
</div>Or add a bio field to the Props interface and profile data.
📝 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.
| <p className='text-sm leading-relaxed text-secondaryText'> | |
| Experienced blockchain researcher specializing in consensus mechanisms and cryptographic protocols. Active contributor to Polkadot ecosystem with focus on runtime | |
| optimization and security research. Published author and speaker at major blockchain conferences. | |
| </p> | |
| <div className='mt-4 border-t border-primary_border pt-4'> | |
| {onChainIdentity?.web && ( | |
| <p className='text-sm leading-relaxed text-secondaryText'> | |
| {onChainIdentity.web} | |
| </p> | |
| )} | |
| </div> |
🤖 Prompt for AI Agents
In src/components/Profile/DashboardHeader.tsx around lines 141 to 144, the
biography is hardcoded placeholder text; change it to render dynamic profile
data by adding a bio field to the component's Props (or using the existing
profile/identity object), update the typing to include bio?: string, and replace
the static paragraph content with the bio prop (with a safe fallback like an
empty string or default message) so the component displays actual profile
biography text.
| const promotionProgress = 75; | ||
| const retentionDeadline = 45; | ||
| const salaryIncrease = 5.2; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace mock data with actual API-driven values.
All key metrics (promotionProgress, retentionDeadline, salaryIncrease) are hardcoded mock data, which limits the component's usefulness.
Consider integrating these values from the API or passing them via props:
interface Props {
address: string;
className?: string;
+ promotionProgress?: number;
+ retentionDeadline?: number;
+ salaryIncrease?: number;
}
- // Mock data for demonstration - in real implementation, these would come from API
- const promotionProgress = 75;
- const retentionDeadline = 45;
- const salaryIncrease = 5.2;
+ const promotionProgress = props.promotionProgress ?? 0;
+ const retentionDeadline = props.retentionDeadline ?? 0;
+ const salaryIncrease = props.salaryIncrease ?? 0;Do you want me to help identify the appropriate API endpoints or data sources for these metrics?
📝 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.
| const promotionProgress = 75; | |
| const retentionDeadline = 45; | |
| const salaryIncrease = 5.2; | |
| // In src/components/Profile/ProfileCards.tsx | |
| interface Props { | |
| address: string; | |
| className?: string; | |
| promotionProgress?: number; | |
| retentionDeadline?: number; | |
| salaryIncrease?: number; | |
| } | |
| const ProfileCards: React.FC<Props> = (props) => { | |
| - // Mock data for demonstration - in real implementation, these would come from API | |
| - const promotionProgress = 75; | |
| - const retentionDeadline = 45; | |
| const promotionProgress = props.promotionProgress ?? 0; | |
| const retentionDeadline = props.retentionDeadline ?? 0; | |
| const salaryIncrease = props.salaryIncrease ?? 0; | |
| // ...rest of component implementation | |
| }; |
🤖 Prompt for AI Agents
In src/components/Profile/ProfileCards.tsx around lines 30 to 32, the values
promotionProgress, retentionDeadline, and salaryIncrease are hardcoded mock
data; replace them by accepting these metrics as props or fetching them from the
appropriate API. Update the component signature to receive
promotionProgress:number, retentionDeadline:number, salaryIncrease:number (or a
single metrics object) and use those props in place of the constants;
alternatively, if the component should fetch its own data, implement a useEffect
hook that calls the API endpoint, stores results in local state, and handles
loading/error states before rendering. Ensure incoming values are validated
(default/fallbacks) and document the prop types or expected API response shape
so the parent can provide correct data.
| const [loading, setLoading] = useState(true); | ||
|
|
||
| // Get GitHub username from fellow's social links or mock data | ||
| const githubUsername = 'mock-user'; // In real implementation, get from fellow's social links |
There was a problem hiding this comment.
Complete the GitHub username retrieval logic.
The component uses a hardcoded mock username instead of extracting it from the fellow's social links as indicated by the comment.
The address prop is passed but not used to retrieve the actual GitHub username. Consider implementing the logic mentioned in the comment:
- // Get GitHub username from fellow's social links or mock data
- const githubUsername = 'mock-user'; // In real implementation, get from fellow's social links
+ const { fellows } = useApiContext();
+ const fellow = fellows.find((f) => f.address === address);
+ const githubUsername = fellow?.social_links?.find((social) => social.type === 'Github')?.link?.split('/').pop() || '';Do you want me to generate a more complete implementation that integrates with the fellow's social links data?
📝 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.
| const githubUsername = 'mock-user'; // In real implementation, get from fellow's social links | |
| // Replace hard-coded mock with actual GitHub username from fellow’s social links | |
| const { fellows } = useApiContext(); | |
| const fellow = fellows.find((f) => f.address === address); | |
| const githubUsername = | |
| fellow | |
| ?.social_links | |
| ?.find((social) => social.type === 'Github') | |
| ?.link | |
| ?.split('/') | |
| .pop() || ''; |
🤖 Prompt for AI Agents
In src/components/Profile/RecentContributions.tsx around line 49, replace the
hardcoded const githubUsername = 'mock-user' with logic that derives the GitHub
username from the fellow's social links using the address prop: lookup the
fellow record (or accept a fellow prop) by address, find the social link whose
provider is 'github' or whose URL matches github.com, extract the username from
the URL or username field, and fall back to null/empty if not found; ensure
proper typing, null checks, and that component renders appropriately when
githubUsername is missing.
| } catch (error) { | ||
| console.error('Error fetching GitHub stats:', error); | ||
| } finally { |
There was a problem hiding this comment.
Display error state to the user.
Errors are only logged to the console without any user-facing feedback, which degrades the user experience when data fetching fails.
Consider adding an error state display:
} catch (error) {
console.error('Error fetching GitHub stats:', error);
+ // Show error message to user
} finally {You could add an error banner or fallback UI within the component's render to inform users when data cannot be loaded.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/components/Profile/RecentContributions.tsx around lines 57 to 59, the
fetch catch block only logs errors to the console so users get no feedback; add
a local error state (e.g., useState<string | null>), set it with a friendly
message inside the catch (or the error.message), and clear it as appropriate;
update the component's JSX to conditionally render an error banner or fallback
UI (brief message and optionally a retry button) when the error state is
non-null, ensuring accessibility (role="alert") and not breaking existing
loading/success UI.
| <button | ||
| onClick={() => setIsAttentionExpanded(!isAttentionExpanded)} | ||
| className='rounded p-1 transition-colors hover:bg-gray-100' | ||
| > | ||
| {isAttentionExpanded ? <Minus className='h-5 w-5 text-secondaryText' /> : <Plus className='h-5 w-5 text-secondaryText' />} | ||
| </button> |
There was a problem hiding this comment.
Add accessibility attributes to the toggle button.
The expand/collapse toggle button lacks proper ARIA attributes, which affects screen reader accessibility.
Apply this diff to improve accessibility:
<button
onClick={() => setIsAttentionExpanded(!isAttentionExpanded)}
className='rounded p-1 transition-colors hover:bg-gray-100'
+ aria-label={isAttentionExpanded ? 'Collapse attention section' : 'Expand attention section'}
+ aria-expanded={isAttentionExpanded}
>
{isAttentionExpanded ? <Minus className='h-5 w-5 text-secondaryText' /> : <Plus className='h-5 w-5 text-secondaryText' />}
</button>📝 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.
| <button | |
| onClick={() => setIsAttentionExpanded(!isAttentionExpanded)} | |
| className='rounded p-1 transition-colors hover:bg-gray-100' | |
| > | |
| {isAttentionExpanded ? <Minus className='h-5 w-5 text-secondaryText' /> : <Plus className='h-5 w-5 text-secondaryText' />} | |
| </button> | |
| <button | |
| onClick={() => setIsAttentionExpanded(!isAttentionExpanded)} | |
| className='rounded p-1 transition-colors hover:bg-gray-100' | |
| aria-label={isAttentionExpanded ? 'Collapse attention section' : 'Expand attention section'} | |
| aria-expanded={isAttentionExpanded} | |
| > | |
| {isAttentionExpanded ? <Minus className='h-5 w-5 text-secondaryText' /> : <Plus className='h-5 w-5 text-secondaryText' />} | |
| </button> |
🤖 Prompt for AI Agents
In src/components/Profile/RequiredActions.tsx around lines 28-33, the
expand/collapse toggle button is missing ARIA attributes and a button type;
update the button to include type="button", set aria-expanded to the
isAttentionExpanded state, and add an aria-controls referencing the id of the
collapsible panel (e.g., "attention-panel"); also provide a clear aria-label (or
aria-labelledby) such as "Toggle attention section". Ensure the controlled panel
element has the matching id ("attention-panel") elsewhere in the component so
screen readers can associate the toggle with the content.
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
src/app/api/v1/address/[address]/activity/utils.ts (1)
120-128: Good defensive fix, but consider type safety.The guard correctly prevents the "IN requires non-empty ArrayValue" Firestore error when no indexes exist, and the downstream code handles the empty
docsarray properly.However, the mock object
{ docs: [] }lacks the fullQuerySnapshottype structure. While this works at runtime (since only.docsis accessed), it may cause TypeScript errors if strict typing is enabled.Consider one of these type-safe alternatives:
Option 1: Explicit typing with a union type
- // Skip Firestore query if no indexes to avoid 'IN requires non-empty ArrayValue' error - let querySnapshot; - if (indexes.length > 0) { - querySnapshot = await postsCollRef(network, ProposalType.FELLOWSHIP_REFERENDUMS).where('index', 'in', indexes).get(); - } else { - // Create empty query snapshot when no indexes - querySnapshot = { docs: [] }; - } + // Skip Firestore query if no indexes to avoid 'IN requires non-empty ArrayValue' error + let querySnapshot: { docs: any[] }; + if (indexes.length > 0) { + querySnapshot = await postsCollRef(network, ProposalType.FELLOWSHIP_REFERENDUMS).where('index', 'in', indexes).get(); + } else { + // Create empty query snapshot when no indexes + querySnapshot = { docs: [] }; + }Option 2: Type assertion (if QuerySnapshot type is imported)
- let querySnapshot; + let querySnapshot: FirebaseFirestore.QuerySnapshot | { docs: [] };Option 3: Restructure to avoid the mock entirely
- // Skip Firestore query if no indexes to avoid 'IN requires non-empty ArrayValue' error - let querySnapshot; - if (indexes.length > 0) { - querySnapshot = await postsCollRef(network, ProposalType.FELLOWSHIP_REFERENDUMS).where('index', 'in', indexes).get(); - } else { - // Create empty query snapshot when no indexes - querySnapshot = { docs: [] }; - } - - const titleContentMap = new Map<string, Metadata>(); - let metadatas = querySnapshot.docs + const titleContentMap = new Map<string, Metadata>(); + + // Skip Firestore query if no indexes to avoid 'IN requires non-empty ArrayValue' error + let metadatas: Metadata[] = []; + if (indexes.length > 0) { + const querySnapshot = await postsCollRef(network, ProposalType.FELLOWSHIP_REFERENDUMS).where('index', 'in', indexes).get(); + metadatas = querySnapshot.docssrc/app/HomeClientWrapper.tsx (1)
35-39: Improve type safety and simplify the comparison.The
anytype assertion and verbose comparison weaken code quality:Apply this diff:
const isFellow = useMemo(() => { if (!loginAddress || !fellows?.length) return false; const substrateAddress = getSubstrateAddress(loginAddress); - return fellows.find((f: any) => f.address === substrateAddress) !== undefined; + return !!fellows.find((f) => f.address === substrateAddress); }, [loginAddress, fellows]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/app/HomeClientWrapper.tsx(1 hunks)src/app/api/v1/address/[address]/activity/utils.ts(1 hunks)src/app/page.tsx(3 hunks)src/components/Profile/index.tsx(1 hunks)src/utils/getEncodedAddress.ts(1 hunks)src/utils/midTruncateText.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/app/api/v1/address/[address]/activity/utils.ts (1)
src/app/api/v1/firestoreRefs.ts (1)
postsCollRef(16-16)
src/components/Profile/index.tsx (1)
src/global/types.ts (1)
IProfile(650-656)
src/app/HomeClientWrapper.tsx (3)
src/global/types.ts (2)
ActivityFeedItem(660-678)IProfile(650-656)src/utils/getSubstrateAddress.ts (1)
getSubstrateAddress(13-21)src/app/api/v1/address/[address]/getProfile.ts (1)
getProfile(19-32)
src/app/page.tsx (2)
src/global/types.ts (1)
ServerComponentProps(91-94)src/app/HomeClientWrapper.tsx (1)
HomeClientWrapper(27-97)
🔇 Additional comments (3)
src/app/page.tsx (1)
8-13: LGTM! Clean refactor to delegate to client wrapper.The refactor properly separates server-side data fetching from client-side rendering logic, which aligns with Next.js best practices for Server Components.
src/components/Profile/index.tsx (2)
20-22: LGTM! Good use of readonly.Making the
profileprop readonly is a best practice that prevents accidental mutations and improves type safety.
29-41: LGTM! Clean modular component structure.The refactored profile layout using smaller, focused components (DashboardHeader, RequiredActions, ProfileCards, RecentContributions) improves maintainability and follows React best practices.
| interface Props { | ||
| feedItems: ActivityFeedItem[] | any[]; | ||
| feed: EActivityFeed; | ||
| network: Network; | ||
| originUrl: string; | ||
| } |
There was a problem hiding this comment.
Replace any[] with a proper type.
The feedItems prop uses any[] as an alternative type, which defeats TypeScript's type safety. Based on line 92's usage, this should be PostFeedListingItem[] or a more specific union type.
Consider defining a proper union type or using a shared base interface:
+import { PostFeedListingItem } from '@/global/types';
+
interface Props {
- feedItems: ActivityFeedItem[] | any[];
+ feedItems: ActivityFeedItem[] | PostFeedListingItem[];
feed: EActivityFeed;
network: Network;
originUrl: string;
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/app/HomeClientWrapper.tsx around lines 20 to 25, replace the loose any[]
in the Props.feedItems type with the correct specific type(s) used at runtime
(e.g. PostFeedListingItem[] or a union like (ActivityFeedItem |
PostFeedListingItem)[]); update the Props interface to use that concrete type,
add or update the necessary import(s) for PostFeedListingItem, and adjust any
call sites or type guards in the file so TypeScript recognizes the union members
rather than falling back to any.
| useEffect(() => { | ||
| if (isFellow && loginAddress) { | ||
| const fetchUserProfile = async () => { | ||
| try { | ||
| setLoading(true); | ||
| const profile = await getProfile({ | ||
| address: loginAddress, | ||
| originUrl, | ||
| network | ||
| }); | ||
| setUserProfile(profile); | ||
| } catch (error) { | ||
| console.error('Error fetching user profile:', error); | ||
| setUserProfile(null); | ||
| } finally { | ||
| setLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| fetchUserProfile(); | ||
| } | ||
| }, [isFellow, loginAddress, network, originUrl]); |
There was a problem hiding this comment.
Add cleanup for userProfile state.
The effect doesn't reset userProfile when isFellow becomes false or when the user logs out. This could display stale profile data.
Add cleanup logic:
useEffect(() => {
+ if (!isFellow) {
+ setUserProfile(null);
+ return;
+ }
+
if (isFellow && loginAddress) {
const fetchUserProfile = async () => {
try {📝 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.
| useEffect(() => { | |
| if (isFellow && loginAddress) { | |
| const fetchUserProfile = async () => { | |
| try { | |
| setLoading(true); | |
| const profile = await getProfile({ | |
| address: loginAddress, | |
| originUrl, | |
| network | |
| }); | |
| setUserProfile(profile); | |
| } catch (error) { | |
| console.error('Error fetching user profile:', error); | |
| setUserProfile(null); | |
| } finally { | |
| setLoading(false); | |
| } | |
| }; | |
| fetchUserProfile(); | |
| } | |
| }, [isFellow, loginAddress, network, originUrl]); | |
| useEffect(() => { | |
| if (!isFellow) { | |
| setUserProfile(null); | |
| return; | |
| } | |
| if (isFellow && loginAddress) { | |
| const fetchUserProfile = async () => { | |
| try { | |
| setLoading(true); | |
| const profile = await getProfile({ | |
| address: loginAddress, | |
| originUrl, | |
| network | |
| }); | |
| setUserProfile(profile); | |
| } catch (error) { | |
| console.error('Error fetching user profile:', error); | |
| setUserProfile(null); | |
| } finally { | |
| setLoading(false); | |
| } | |
| }; | |
| fetchUserProfile(); | |
| } | |
| }, [isFellow, loginAddress, network, originUrl]); |
🤖 Prompt for AI Agents
In src/app/HomeClientWrapper.tsx around lines 42 to 63, the effect that fetches
the profile doesn't clear userProfile when isFellow becomes false or on cleanup,
which can leave stale data visible; update the effect so that when isFellow is
false (or on cleanup/unmount) you setUserProfile(null) and cancel any in-flight
fetch (use an abort flag or AbortController) to avoid racing updates, and ensure
setLoading is set appropriately in the cleanup path.
| } | ||
| }, [isFellow, loginAddress, network, originUrl]); | ||
|
|
||
| console.log('userProfile', userProfile); |
There was a problem hiding this comment.
Remove debug console.log statement.
Debug logging statements should not remain in production code, especially when logging user profile data that may contain PII.
Apply this diff:
- console.log('userProfile', userProfile);
-📝 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.
| console.log('userProfile', userProfile); |
🤖 Prompt for AI Agents
In src/app/HomeClientWrapper.tsx at line 65, remove the debug
console.log('userProfile', userProfile); statement which prints user profile
data; instead delete the line (or replace with a non-PII-safe logger if
absolutely required) so no user PII is logged to the console in production.
| return ( | ||
| <div className='flex w-full flex-col gap-y-8'> | ||
| <Carousel /> | ||
|
|
||
| <div className='mb-16 flex flex-col items-center gap-8 md:mb-auto lg:flex-row lg:items-start'> | ||
| <div className='flex w-full flex-col gap-y-4'> | ||
| <Stats className='lg:hidden' /> | ||
| <ActivitySelectorCard value={feed} /> | ||
| {feed === EActivityFeed.ALL ? <ActivityFeed items={feedItems as ActivityFeedItem[]} /> : <PostFeed items={feedItems as any[]} />} | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
Remove unsafe type assertion.
Line 92 uses as any[] which bypasses all type checking and can hide bugs. This should use a proper type that matches the PostFeed component's expected props.
After fixing the Props interface type (see earlier comment), update line 92:
- {feed === EActivityFeed.ALL ? <ActivityFeed items={feedItems as ActivityFeedItem[]} /> : <PostFeed items={feedItems as any[]} />}
+ {feed === EActivityFeed.ALL ? <ActivityFeed items={feedItems as ActivityFeedItem[]} /> : <PostFeed items={feedItems as PostFeedListingItem[]} />}📝 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.
| return ( | |
| <div className='flex w-full flex-col gap-y-8'> | |
| <Carousel /> | |
| <div className='mb-16 flex flex-col items-center gap-8 md:mb-auto lg:flex-row lg:items-start'> | |
| <div className='flex w-full flex-col gap-y-4'> | |
| <Stats className='lg:hidden' /> | |
| <ActivitySelectorCard value={feed} /> | |
| {feed === EActivityFeed.ALL ? <ActivityFeed items={feedItems as ActivityFeedItem[]} /> : <PostFeed items={feedItems as any[]} />} | |
| </div> | |
| </div> | |
| </div> | |
| ); | |
| return ( | |
| <div className='flex w-full flex-col gap-y-8'> | |
| <Carousel /> | |
| <div className='mb-16 flex flex-col items-center gap-8 md:mb-auto lg:flex-row lg:items-start'> | |
| <div className='flex w-full flex-col gap-y-4'> | |
| <Stats className='lg:hidden' /> | |
| <ActivitySelectorCard value={feed} /> | |
| {feed === EActivityFeed.ALL ? <ActivityFeed items={feedItems as ActivityFeedItem[]} /> : <PostFeed items={feedItems as PostFeedListingItem[]} />} | |
| </div> | |
| </div> | |
| </div> | |
| ); |
🤖 Prompt for AI Agents
In src/app/HomeClientWrapper.tsx around lines 84 to 96, line 92 currently uses
an unsafe "as any[]" assertion for PostFeed items; remove that assertion and
instead pass feedItems with a concrete typed array that matches PostFeed's props
(e.g., Post[] or the specific PostFeedItem type). Update the PostFeed props
interface (or the feedItems variable declaration) so its items parameter is
correctly typed, then replace "feedItems as any[]" with "feedItems" (or
"feedItems as Post[]" if an explicit cast is temporarily needed) so TypeScript
enforces the correct shape.
| return ( | ||
| <div className='flex w-full flex-col gap-y-8'> | ||
| <Carousel /> | ||
|
|
||
| <div className='mb-16 flex flex-col items-center gap-8 md:mb-auto lg:flex-row lg:items-start'> | ||
| <div className='flex w-full flex-col gap-y-4 lg:max-w-[calc(100%-300px)]'> | ||
| <PendingTasks className='md:hidden' /> | ||
| <Stats className='lg:hidden' /> | ||
| <ActivitySelectorCard value={feed as EActivityFeed} /> | ||
| {feed === EActivityFeed.ALL ? <ActivityFeed items={(feedItems || []) as ActivityFeedItem[]} /> : <PostFeed items={(feedItems || []) as PostFeedListingItem[]} />} | ||
| </div> | ||
| <div className='flex w-full flex-col gap-y-4 lg:w-[300px]'> | ||
| <PendingTasks className='hidden md:flex' /> | ||
| <Stats className='hidden lg:flex' /> | ||
| <JoinFellowshipCard /> | ||
| <TrendingProposals proposals={trending} /> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| <HomeClientWrapper | ||
| feedItems={feedItems} | ||
| feed={feed as EActivityFeed} | ||
| network={network as Network} | ||
| originUrl={originUrl} | ||
| /> | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
Add validation for the network parameter.
The feed parameter is validated (lines 29-31), but network is cast to Network type without validation. If an invalid network value is passed, this could cause runtime errors in downstream components.
Add network validation similar to feed:
// validate feed search param
if (feed && !Object.values(EActivityFeed).includes(feed as EActivityFeed)) {
throw new ClientError(MESSAGES.INVALID_SEARCH_PARAMS_ERROR, API_ERROR_CODE.INVALID_SEARCH_PARAMS_ERROR);
}
+
+ // validate network search param
+ if (network && !Object.values(Network).includes(network as Network)) {
+ throw new ClientError(MESSAGES.INVALID_SEARCH_PARAMS_ERROR, API_ERROR_CODE.INVALID_SEARCH_PARAMS_ERROR);
+ }Verify that the Network enum/type supports iteration with Object.values():
🏁 Script executed:
#!/bin/bash
# Description: Check the Network type definition to ensure it can be validated
# Search for Network type/enum definition
ast-grep --pattern 'export enum Network {
$$$
}'
# Also search for type union definitions
rg -nP 'export (type|interface) Network' --type=ts -A 5Length of output: 725
🏁 Script executed:
#!/bin/bash
sed -n '1,60p' src/app/page.tsxLength of output: 1628
Add validation for the network parameter.
The feed parameter is validated (lines 29-31), but network is cast to Network type without validation. The Network enum (src/global/types.ts, lines 16-19) supports Object.values(), so the suggested validation approach is sound. Adding this check ensures invalid network values are caught at parse time rather than causing downstream errors.
Suggested fix:
// validate feed search param
if (feed && !Object.values(EActivityFeed).includes(feed as EActivityFeed)) {
throw new ClientError(MESSAGES.INVALID_SEARCH_PARAMS_ERROR, API_ERROR_CODE.INVALID_SEARCH_PARAMS_ERROR);
}
+
+ // validate network search param
+ if (network && !Object.values(Network).includes(network as Network)) {
+ throw new ClientError(MESSAGES.INVALID_SEARCH_PARAMS_ERROR, API_ERROR_CODE.INVALID_SEARCH_PARAMS_ERROR);
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/app/page.tsx around lines 38 to 45, add validation for the network param
like the existing feed validation (lines 29-31): check that the parsed network
value is one of Object.values(Network) and if it is invalid, call notFound() (or
otherwise short-circuit similarly to feed) instead of casting to Network; update
the prop passed to HomeClientWrapper only after the validation so downstream
code receives a guaranteed-valid Network value.
| {/* <ContributionGraph | ||
| classNames='mt-[56px]' | ||
| githubUsername={socialLinks?.find((social) => social.type === 'Github')?.link || ''} | ||
| openProfileEdit={openProfileEdit} | ||
| /> | ||
| <div className='mt-4 flex flex-col gap-4 md:grid md:grid-cols-11'> | ||
| /> */} | ||
| {/* <div className='mt-4 flex flex-col gap-4 md:grid md:grid-cols-11'> | ||
| <section className='flex w-full flex-col gap-4 md:col-span-5'> | ||
| <Manifesto | ||
| manifesto={manifesto} | ||
| address={address} | ||
| /> | ||
| <PromotionDetails address={address} /> | ||
| </section> | ||
| <section className='w-full md:col-span-6'> | ||
| <ProfileProposals address={address} /> | ||
| </section> | ||
| </div> | ||
| <div className='mt-4 pb-4'> | ||
| </div> */} |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove commented code blocks.
Large blocks of commented code (ContributionGraph at lines 42-46, and Manifesto/ProfileProposals at lines 47-57) should either be:
- Removed entirely if the features are no longer needed
- Un-commented and integrated if they're still required
Leaving commented code creates confusion and maintenance burden.
If these features are deprecated, remove them:
- {/* <ContributionGraph
- classNames='mt-[56px]'
- githubUsername={socialLinks?.find((social) => social.type === 'Github')?.link || ''}
- openProfileEdit={openProfileEdit}
- /> */}
- {/* <div className='mt-4 flex flex-col gap-4 md:grid md:grid-cols-11'>
- <section className='flex w-full flex-col gap-4 md:col-span-5'>
- <Manifesto
- manifesto={manifesto}
- address={address}
- />
- </section>
- <section className='w-full md:col-span-6'>
- <ProfileProposals address={address} />
- </section>
- </div> */}If you need these features restored, I can help integrate them properly.
📝 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.
| {/* <ContributionGraph | |
| classNames='mt-[56px]' | |
| githubUsername={socialLinks?.find((social) => social.type === 'Github')?.link || ''} | |
| openProfileEdit={openProfileEdit} | |
| /> | |
| <div className='mt-4 flex flex-col gap-4 md:grid md:grid-cols-11'> | |
| /> */} | |
| {/* <div className='mt-4 flex flex-col gap-4 md:grid md:grid-cols-11'> | |
| <section className='flex w-full flex-col gap-4 md:col-span-5'> | |
| <Manifesto | |
| manifesto={manifesto} | |
| address={address} | |
| /> | |
| <PromotionDetails address={address} /> | |
| </section> | |
| <section className='w-full md:col-span-6'> | |
| <ProfileProposals address={address} /> | |
| </section> | |
| </div> | |
| <div className='mt-4 pb-4'> | |
| </div> */} |
🤖 Prompt for AI Agents
In src/components/Profile/index.tsx around lines 42 to 57 there are two large
commented blocks (ContributionGraph and the Manifesto/ProfileProposals layout)
left in the JSX; remove those commented code blocks entirely if the features are
deprecated, or fully restore/uncomment and integrate them properly into the
component if they are still needed. Ensure the resulting JSX has no commented
fragments, update imports (remove unused imports if you delete the blocks or
add/verify imports if you restore them), and run a quick lint/type check to
confirm no unused variables or JSX errors remain.
| } | ||
|
|
||
| if (address.startsWith('0x')) return address; | ||
| if (address?.startsWith?.('0x')) return address; |
There was a problem hiding this comment.
Type signature doesn't match defensive runtime check.
The function signature declares address: string (non-nullable), but line 22 uses double optional chaining: address?.startsWith?.('0x'). This defends against:
addressbeingnull/undefinedstartsWithmethod not existing
However, if address is typed as string, both checks are unnecessary—startsWith is a standard string method.
Recommendations:
-
If
addresscan benull/undefined, update the signature:-export default function getEncodedAddress(address: string, network: Network): string | null { +export default function getEncodedAddress(address: string | null | undefined, network: Network): string | null {
-
If
addressis guaranteed to be a string, remove the optional chaining:- if (address?.startsWith?.('0x')) return address; + if (address.startsWith('0x')) return address;
🤖 Prompt for AI Agents
In src/utils/getEncodedAddress.ts around line 22: the function is typed as
accepting a non-nullable string but uses optional chaining
(address?.startsWith?.('0x')), which is inconsistent; either make the parameter
nullable (e.g., string | null | undefined) and handle the null case explicitly
(and update return type) or, if address is always a string, remove the optional
chaining and call address.startsWith('0x') directly; pick one approach and
update the function signature and any callers accordingly so types and runtime
checks match.
| if (!text) return ''; | ||
| const maxLength = startChars + endChars; | ||
|
|
||
| return text.length > maxLength ? `${text.slice(0, startChars)}${separator}${text.slice(-endChars)}` : text; | ||
| return text?.length > maxLength ? `${text?.slice(0, startChars)}${separator}${text?.slice(-endChars)}` : text; |
There was a problem hiding this comment.
Type signature doesn't match defensive runtime checks.
The function signature declares text: string (non-nullable), but the implementation adds:
- Line 21: An early return for falsy
text - Line 24: Optional chaining on
text?.lengthandtext?.slice
This creates a type/runtime mismatch. Additionally, the optional chaining on line 24 is redundant—if text is falsy, line 21 already returns early, so line 24 will only execute when text is truthy.
Recommendations:
-
If
textcan benull/undefined, update the signature to reflect that:-export default function midTruncateText({ text, startChars, endChars, separator = '...' }: Params): string { +export default function midTruncateText({ text, startChars, endChars, separator = '...' }: Omit<Params, 'text'> & { text: string | null | undefined }): string {
Or update the
Paramsinterface to maketextoptional. -
If
textis guaranteed to be a non-null string, remove the defensive checks:- if (!text) return ''; - return text?.length > maxLength ? `${text?.slice(0, startChars)}${separator}${text?.slice(-endChars)}` : text; + return text.length > maxLength ? `${text.slice(0, startChars)}${separator}${text.slice(-endChars)}` : text;
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/utils/midTruncateText.ts around lines 21 to 24, the function signature
declares text as a non-nullable string but the implementation treats it as
possibly falsy (early return and optional chaining), causing a mismatch; update
the Params/interface so text is optional or typed as string | null | undefined
(e.g., text?: string or text: string | null | undefined), keep the existing
early-return defensive check, and remove the redundant optional chaining on line
24 (use text.length and text.slice directly after the null-check).
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (3)
src/components/Profile/RequiredActions.tsx (1)
28-33: Accessibility issue flagged in previous review.The toggle button is missing accessibility attributes (
aria-label,aria-expanded,aria-controls) and thetype="button"attribute. This was already identified in a previous review comment.src/components/Header/RightSidebar/ActivityFeed.tsx (2)
21-86: Typefellowsprecisely (Fellow[]) and avoidany.Use a proper
Fellowtype forgetActivityDetailsto regain type-safety and editor help.-import { ActivityFeedItem, EActivityFeed, SubsquidActivityType } from '@/global/types'; +import { ActivityFeedItem, EActivityFeed, SubsquidActivityType, type Fellow } from '@/global/types'; ... -const getActivityDetails = (feedItem: ActivityFeedItem, fellows: any[] = []) => { +const getActivityDetails = (feedItem: ActivityFeedItem, fellows: Fellow[] = []) => {
108-112:toSortednot supported on Node 18 (SSR crash).This was flagged earlier and still present. Switch to
slice().sort(...).slice(0, 5).-const sortedFeedItems = feedItems.toSorted((a, b) => new Date(b.created_at).getTime() - new Date(a.created_at).getTime()).slice(0, 5); +const sortedFeedItems = feedItems + .slice() + .sort((a, b) => new Date(b.created_at).getTime() - new Date(a.created_at).getTime()) + .slice(0, 5);
🧹 Nitpick comments (8)
src/components/Profile/RequiredActions.tsx (1)
36-99: Consider adding empty state and conditional rendering.The component always displays all three action blocks when expanded, even if no actions are required (e.g., zero pending proposals). This could confuse users.
Consider:
- Conditionally render each block only when the count is > 0
- Display an empty state when no actions are required:
{isAttentionExpanded && ( <CardBody> {(pendingProposalsCount === 0 && juniorMembersCount === 0 && !evidenceRequired) ? ( <p className='text-center text-text_secondary py-4'> No actions required at this time </p> ) : ( <div className='space-y-4'> {evidenceRequired && <div>...</div>} {pendingProposalsCount > 0 && <div>...</div>} {juniorMembersCount > 0 && <div>...</div>} </div> )} </CardBody> )}src/components/Header/AppSidebar.tsx (2)
80-84: Remove commented code if Profile navigation is no longer needed.Leaving commented code can cause confusion. If the Profile navigation item is planned for future use, consider tracking it via a TODO comment with context or an issue.
- // { - // label: 'Profile', - // icon: Shield, - // url: '/address' - // },
138-138: Remove commented JoinFellowshipButton if no longer needed.Similar to the commented Profile navigation item, this should be removed to keep the codebase clean. If it's planned for future use, track it properly with a TODO or issue reference.
-{/* <JoinFellowshipButton className='mb-5' /> */}src/components/Home/ActivitySelectorCard.tsx (2)
69-77: Controlled component: drop redundantdefaultValue.You already control with
value. RemovedefaultValueto avoid confusion.- defaultValue={value.toString()} value={value.toString()}
25-44: Scroll animation: tighten and add cleanup.5ms intervals are costly and there’s no cleanup on unmount. Prefer
requestAnimationFrameor at least store/clear the timer via a ref +useEffectcleanup.Would you like a small refactor using
requestAnimationFrameandAbortControllerfor cancellation?src/app/(site)/activity/ActivityClientWrapper.tsx (1)
12-20: Remove unused props (network,originUrl).They aren’t used inside the component; keeping them widens the surface and invites drift. Trim here and at call sites.
-interface Props { - feedItems: ActivityFeedItem[]; - feed: EActivityFeed; - network: Network; - originUrl: string; -} +interface Props { + feedItems: ActivityFeedItem[]; + feed: EActivityFeed; +} ... -export default function ActivityClientWrapper({ feedItems, feed, network, originUrl }: Props) { +export default function ActivityClientWrapper({ feedItems, feed }: Props) {src/app/(site)/activity/page.tsx (2)
25-41: Also validatenetworksearch param (defensive).
feedis validated;networkis blindly cast. Validate against allowed networks or omit when invalid to prevent badx-networkheaders downstream.If
Networkis a union/enum, add a guard like:const safeNetwork = Object.values(Network as any).includes(network as any) ? (network as Network) : undefined;Then pass
safeNetworktogetActivityFeed.
42-49: If trimming ClientWrapper props, stop passing unused values.Pairs with the earlier suggestion to remove
network/originUrlfrom ActivityClientWrapper.- <ActivityClientWrapper - feedItems={feedItems as ActivityFeedItem[]} - feed={feed as EActivityFeed} - network={network as Network} - originUrl={originUrl} - /> + <ActivityClientWrapper + feedItems={feedItems as ActivityFeedItem[]} + feed={feed as EActivityFeed} + />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/app/(site)/activity/ActivityClientWrapper.tsx(1 hunks)src/app/(site)/activity/page.tsx(1 hunks)src/components/Header/AppSidebar.tsx(5 hunks)src/components/Header/RightSidebar/ActivityFeed.tsx(1 hunks)src/components/Home/ActivitySelectorCard.tsx(2 hunks)src/components/Profile/RequiredActions.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
src/components/Header/RightSidebar/ActivityFeed.tsx (4)
src/global/types.ts (1)
ActivityFeedItem(660-678)src/utils/getSubstrateAddress.ts (1)
getSubstrateAddress(13-21)src/utils/getOriginUrl.ts (1)
getOriginUrl(7-12)src/app/api/v1/feed/getActivityFeed.ts (1)
getActivityFeed(20-34)
src/app/(site)/activity/page.tsx (6)
src/global/types.ts (2)
ServerComponentProps(91-94)ActivityFeedItem(660-678)src/global/exceptions.ts (1)
ClientError(9-14)src/global/constants/errorCodes.ts (1)
API_ERROR_CODE(5-18)src/utils/getOriginUrl.ts (1)
getOriginUrl(7-12)src/app/api/v1/feed/getActivityFeed.ts (1)
getActivityFeed(20-34)src/app/(site)/activity/ActivityClientWrapper.tsx (1)
ActivityClientWrapper(19-26)
src/app/(site)/activity/ActivityClientWrapper.tsx (2)
src/global/types.ts (1)
ActivityFeedItem(660-678)src/components/Header/RightSidebar/ActivityFeed.tsx (1)
ActivityFeed(88-198)
src/components/Home/ActivitySelectorCard.tsx (1)
src/utils/getSubstrateAddress.ts (1)
getSubstrateAddress(13-21)
src/components/Header/AppSidebar.tsx (1)
src/utils/getSubstrateAddress.ts (1)
getSubstrateAddress(13-21)
🔇 Additional comments (2)
src/components/Header/AppSidebar.tsx (2)
7-7: LGTM! Icon library migration and utility imports are correct.The imports align with the refactoring to component-based icons and fellowship-aware navigation.
Also applies to: 12-12, 15-15
17-30: LGTM! Icon rendering is properly guarded.The function correctly handles optional icon components and applies conditional styling.
| const isFellow = useMemo(() => { | ||
| if (!id || !loginAddress || !fellows?.length) return false; | ||
| const substrateAddress = getSubstrateAddress(loginAddress); | ||
| return fellows.some((f: any) => f.address === substrateAddress); | ||
| }, [loginAddress, fellows]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace any type with proper Fellow type definition.
The any type bypasses TypeScript's type safety. Define or import a proper type for fellow objects to catch potential errors at compile time.
const isFellow = useMemo(() => {
if (!id || !loginAddress || !fellows?.length) return false;
const substrateAddress = getSubstrateAddress(loginAddress);
- return fellows.some((f: any) => f.address === substrateAddress);
+ return fellows.some((f) => f.address === substrateAddress);
}, [loginAddress, fellows]);Ensure the fellows array from useApiContext has a proper type definition. If a Fellow type doesn't exist, consider adding one:
type Fellow = {
address: string;
// ... other properties
};🤖 Prompt for AI Agents
In src/components/Header/AppSidebar.tsx around lines 99 to 103, the useMemo
callback uses "any" for fellow objects which bypasses TypeScript safety; replace
the "any" with a proper Fellow type (either import an existing Fellow interface
or define one in a shared types file, e.g. type Fellow = { address: string; /*
other props */ }) and update the fellows array type in useApiContext to Fellow[]
so the .some callback becomes (f: Fellow) => f.address === substrateAddress;
adjust any related imports and type annotations accordingly.
Add id to the useMemo dependency array.
The isFellow computation references id in its condition but doesn't include it in the dependency array, which can lead to stale values.
-}, [loginAddress, fellows]);
+}, [id, loginAddress, fellows]);📝 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.
| const isFellow = useMemo(() => { | |
| if (!id || !loginAddress || !fellows?.length) return false; | |
| const substrateAddress = getSubstrateAddress(loginAddress); | |
| return fellows.some((f: any) => f.address === substrateAddress); | |
| }, [loginAddress, fellows]); | |
| const isFellow = useMemo(() => { | |
| if (!id || !loginAddress || !fellows?.length) return false; | |
| const substrateAddress = getSubstrateAddress(loginAddress); | |
| return fellows.some((f: any) => f.address === substrateAddress); | |
| }, [id, loginAddress, fellows]); |
🤖 Prompt for AI Agents
In src/components/Header/AppSidebar.tsx around lines 99 to 103, the useMemo for
isFellow references the variable id but does not include it in the dependency
array, which can cause stale results; update the dependency array to include id
(i.e., [id, loginAddress, fellows]) so the memo recalculates when id changes and
ensure the dependency list reflects all referenced values.
| const handleOnValueChange = (activityValue: string) => { | ||
| router.push(`/?feed=${activityValue}&network=${network}`); | ||
| router.push(isFellow ? `/activity?feed=${activityValue}&network=${network}` : `/?feed=${activityValue}&network=${network}`); | ||
| }; |
There was a problem hiding this comment.
Avoid pushing network=undefined in query.
If network is unset, the URL becomes ...?network=undefined. Append network only when defined.
- router.push(isFellow ? `/activity?feed=${activityValue}&network=${network}` : `/?feed=${activityValue}&network=${network}`);
+ const q = `feed=${encodeURIComponent(activityValue)}${network ? `&network=${encodeURIComponent(network)}` : ''}`;
+ router.push(isFellow ? `/activity?${q}` : `/?${q}`);📝 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.
| const handleOnValueChange = (activityValue: string) => { | |
| router.push(`/?feed=${activityValue}&network=${network}`); | |
| router.push(isFellow ? `/activity?feed=${activityValue}&network=${network}` : `/?feed=${activityValue}&network=${network}`); | |
| }; | |
| const handleOnValueChange = (activityValue: string) => { | |
| const q = `feed=${encodeURIComponent(activityValue)}${network ? `&network=${encodeURIComponent(network)}` : ''}`; | |
| router.push(isFellow ? `/activity?${q}` : `/?${q}`); | |
| }; |
🤖 Prompt for AI Agents
In src/components/Home/ActivitySelectorCard.tsx around lines 46 to 48, the
handler always appends network which results in URLs like ?network=undefined
when network is unset; change the push to only include the network query param
when network is defined — build the target URL (or query object) conditionally
(e.g. include `&network=${network}` only if network != null/undefined or
construct a query object and omit network when falsy) and call router.push with
that cleaned URL/query so no `network=undefined` appears.
| // Check if current user is a fellow | ||
| const isFellow = useMemo(() => { | ||
| if (!id || !loginAddress || !fellows?.length) return false; | ||
| const substrateAddress = getSubstrateAddress(loginAddress); | ||
| return fellows.some((f: any) => f.address === substrateAddress); | ||
| }, [loginAddress, fellows]); | ||
|
|
There was a problem hiding this comment.
Memo deps + typing + address normalization.
- Add
idto deps to avoid staleisFellow. - Replace
anywithFellow. - Normalize both addresses before compare to avoid prefix mismatches.
-import { EActivityFeed } from '@/global/types';
+import { EActivityFeed, type Fellow } from '@/global/types';
...
-const isFellow = useMemo(() => {
- if (!id || !loginAddress || !fellows?.length) return false;
- const substrateAddress = getSubstrateAddress(loginAddress);
- return fellows.some((f: any) => f.address === substrateAddress);
-}, [loginAddress, fellows]);
+const isFellow = useMemo(() => {
+ if (!id || !loginAddress || !fellows?.length) return false;
+ const loginSub = getSubstrateAddress(loginAddress);
+ if (!loginSub) return false;
+ return fellows.some((f: Fellow) => getSubstrateAddress(f.address) === loginSub);
+}, [id, loginAddress, fellows]);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/components/Home/ActivitySelectorCard.tsx around lines 50 to 56, the
useMemo for isFellow is missing the id dependency, uses any for fellow typing,
and compares addresses without normalizing which can cause prefix mismatches;
update the fellow type from any to Fellow, add id to the dependency array, and
when comparing addresses call getSubstrateAddress on both sides (e.g., normalize
f.address and loginAddress/substrateAddress) so comparisons are robust.
| <div className='flex items-center gap-3'> | ||
| <CheckCircle2 className='h-5 w-5 text-blue-600' /> | ||
| <div> | ||
| <h4 className='font-semibold text-black dark:text-white'>Vote needed on 2 pending proposals</h4> |
There was a problem hiding this comment.
Replace hardcoded counts with dynamic data.
The component displays hardcoded values ("2 pending proposals", "4 junior members") that will present stale or incorrect information to users.
Consider one of these approaches:
- Accept data via props:
interface Props {
className?: string;
+ pendingProposalsCount: number;
+ juniorMembersCount: number;
}
-function RequiredActions({ className }: Props) {
+function RequiredActions({ className, pendingProposalsCount, juniorMembersCount }: Props) {- Fetch data within the component:
const [counts, setCounts] = useState({ proposals: 0, members: 0 });
useEffect(() => {
// Fetch from API
fetchRequiredActionsCounts().then(setCounts);
}, []);Then use the dynamic values:
-<h4 className='font-semibold text-black dark:text-white'>Vote needed on 2 pending proposals</h4>
+<h4 className='font-semibold text-black dark:text-white'>Vote needed on {pendingProposalsCount} pending proposals</h4>-<h4 className='font-semibold text-black dark:text-white'>Consider grading 4 junior members</h4>
+<h4 className='font-semibold text-black dark:text-white'>Consider grading {juniorMembersCount} junior members</h4>Also applies to: 83-83
🤖 Prompt for AI Agents
In src/components/Profile/RequiredActions.tsx around lines 63 and 83, the
component currently renders hardcoded counts ("2 pending proposals", "4 junior
members"); replace these static values with dynamic data by either accepting
counts as props (e.g., props.requiredActionCounts.proposals / .members) or
fetching them inside the component into state (useState for counts and useEffect
to call an API helper then set state), and then render those state/prop values
instead of the hardcoded numbers; ensure loading/default values are handled (0
or a spinner) and update both occurrences on lines 63 and 83.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/components/Header/AppSidebar.tsx (1)
103-107: Address the flagged issues and add defensive null check.The past review comments on these lines remain valid:
- Replace
anytype with a proper Fellow type definition- Add
idto the useMemo dependency arrayAdditionally,
getSubstrateAddresscan returnnull, which isn't explicitly handled. While the current code would returnfalse(correct behavior), it's clearer and more defensive to check explicitly.Apply this diff to address all issues:
const isFellow = useMemo(() => { if (!id || !loginAddress || !fellows?.length) return false; const substrateAddress = getSubstrateAddress(loginAddress); + if (!substrateAddress) return false; - return fellows.some((f: any) => f.address === substrateAddress); + return fellows.some((f) => f.address === substrateAddress); -}, [loginAddress, fellows]); +}, [id, loginAddress, fellows]);Ensure the
fellowsarray has a proper type definition (e.g.,Fellow[]whereFellowincludes at least anaddress: stringproperty).
🧹 Nitpick comments (1)
src/components/Header/AppSidebar.tsx (1)
142-142: Consider removing commented-out code.The
JoinFellowshipButtonis commented out. If this component is no longer needed, consider removing the commented code entirely to keep the codebase clean. If it might be needed in the future, consider adding a TODO comment explaining why it's preserved.-{/* <JoinFellowshipButton className='mb-5' /> */}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Header/AppSidebar.tsx(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/Header/AppSidebar.tsx (1)
src/utils/getSubstrateAddress.ts (1)
getSubstrateAddress(13-21)
🔇 Additional comments (6)
src/components/Header/AppSidebar.tsx (6)
7-7: LGTM! Imports align with functionality changes.The added imports (
useMemo,getSubstrateAddress, and icon components from lucide-react) are appropriate for the functionality changes in this file.Also applies to: 12-12, 15-15
17-30: LGTM! Component type signature is correct.The updated
ListboxItemStartContentcomponent correctly accepts and renders icons as React components with appropriate conditional styling.
32-38: LGTM! Type definition aligns with component icon usage.The
NavItemtype correctly definesiconas an optional React component, which is consistent with the icon component imports and usage throughout the file.
110-110: LGTM! Dependency array is correct.The
navItemsmemoization correctly depends onisFellowandloginAddress, matching the parameters passed togetNavItems.
144-202: LGTM! Navigation rendering logic is sound.The Listbox rendering correctly handles route matching, navigation actions, and conditional styling for current routes and sub-items.
45-94: Profile item is accessible to fellows via Footer navigation—verify if the AppSidebar conditional is intentional.While the Profile item is hidden from AppSidebar for fellows (
!isFellow && loginAddress), it remains available in the Footer's static navigation for all users, including fellows. This allows fellows to access their profile page via the footer. Confirm whether this inconsistency between sidebar and footer is intentional or if Profile should be consistently shown/hidden across both navigation components.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/components/CreateProposal/CreateProposalForm.tsx (1)
46-72: Form validity still ignores field validationThe past review comment correctly identified that
isFormValiddoesn't include react-hook-form's validation state. Line 67 still computes validity based only on wallet/address/API readiness, allowing the parent to enable submission buttons even when required fields fail validation rules defined in the Controllers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/CreateProposal/CreateProposalForm.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/CreateProposal/CreateProposalForm.tsx (1)
src/utils/getSubstrateAddress.ts (1)
getSubstrateAddress(13-21)
| const [error, setError] = useState(''); | ||
| const [selectedWallet, setSelectedWallet] = useState<Wallet | null>(null); | ||
| const [selectedAddress, setSelectedAddress] = useState<InjectedAccount | null>(null); | ||
| const [txStatus, setTxStatus] = useState(''); |
There was a problem hiding this comment.
Transaction status state is never populated
txStatus is declared at line 64 and displayed at lines 430-444, but it's only cleared to an empty string at line 211 and never set to an actual status message. The UI block will never render meaningful content.
Do you want me to wire up the transaction status messages (e.g., "Signing transaction...", "Broadcasting...") or should this be removed until the actual transaction logic is implemented?
Also applies to: 211-211, 430-444
| onFormStateChange?.(isFormValid, loading); | ||
| }, [isFormValid, loading, onFormStateChange]); | ||
|
|
||
| const currentFellow = fellows?.find((fellow) => fellow.address === getSubstrateAddress(addresses?.[0] || '')); |
There was a problem hiding this comment.
Current fellow lookup may show wrong rank
Line 74 computes currentFellow from addresses[0], but the user may have selected a different address via the AddressSwitch. When selectedAddress differs from addresses[0], the displayed rank at lines 236-242 will be incorrect.
Apply this diff to use the selected address:
-const currentFellow = fellows?.find((fellow) => fellow.address === getSubstrateAddress(addresses?.[0] || ''));
+const currentFellow = selectedAddress?.address
+ ? fellows?.find((fellow) => fellow.address === getSubstrateAddress(selectedAddress.address))
+ : null;📝 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.
| const currentFellow = fellows?.find((fellow) => fellow.address === getSubstrateAddress(addresses?.[0] || '')); | |
| const currentFellow = selectedAddress?.address | |
| ? fellows?.find((fellow) => fellow.address === getSubstrateAddress(selectedAddress.address)) | |
| : null; |
🤖 Prompt for AI Agents
In src/components/CreateProposal/CreateProposalForm.tsx around line 74, the
currentFellow lookup uses addresses[0] which can be wrong if the user selected a
different address via AddressSwitch; change the lookup to use the
selectedAddress when present (fallback to addresses[0] or empty string) so
currentFellow = fellows?.find(f => f.address ===
getSubstrateAddress(selectedAddress || addresses?.[0] || '')); this ensures the
displayed rank uses the actively selected address.
| const validateFormData = (data: FormData): string | null => { | ||
| if (!data.title || data.title.trim().length === 0) { | ||
| return 'Title is required.'; | ||
| } | ||
|
|
||
| if (data.title.trim().length < 5) { | ||
| return 'Title must be at least 5 characters long.'; | ||
| } | ||
|
|
||
| if (data.title.trim().length > 200) { | ||
| return 'Title must be less than 200 characters.'; | ||
| } | ||
|
|
||
| if (!data.proposalType || !Object.values(ProposalType).includes(data.proposalType)) { | ||
| return 'Please select a valid proposal type.'; | ||
| } | ||
|
|
||
| if (!data.motivation || data.motivation.trim().length === 0) { | ||
| return 'Motivation is required.'; | ||
| } | ||
|
|
||
| if (data.motivation.trim().length < 20) { | ||
| return 'Motivation must be at least 20 characters long.'; | ||
| } | ||
|
|
||
| if (data.motivation.trim().length > 5000) { | ||
| return 'Motivation must be less than 5,000 characters.'; | ||
| } | ||
|
|
||
| if (!data.interest || data.interest.trim().length === 0) { | ||
| return 'Interest is required.'; | ||
| } | ||
|
|
||
| if (data.interest.trim().length < 10) { | ||
| return 'Interest must be at least 10 characters long.'; | ||
| } | ||
|
|
||
| if (data.interest.trim().length > 2000) { | ||
| return 'Interest must be less than 2,000 characters.'; | ||
| } | ||
|
|
||
| if (!data.evidenceId || data.evidenceId.trim().length === 0) { | ||
| return 'Evidence is required.'; | ||
| } | ||
|
|
||
| return null; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove duplicate validation logic
The validateFormData function duplicates all validation rules already declared in the Controller rules props (lines 251-254, 287, 332-335, 370-373, 404). Once react-hook-form validation is properly integrated (per the past comment), this manual function becomes redundant and creates a maintenance burden.
After integrating formState.isValid as suggested in the past comment, remove validateFormData and the call at lines 177-181. React-hook-form will handle all validation automatically.
🤖 Prompt for AI Agents
In src/components/CreateProposal/CreateProposalForm.tsx around lines 77 to 123,
the validateFormData function duplicates validation already declared in each
Controller's rules and should be removed; delete the entire validateFormData
function and also remove its invocation at lines 177-181, and ensure you rely on
react-hook-form's formState.isValid (with proper mode: 'onChange' or similar) to
gate submission so all validation is handled by Controller rules rather than
manual checks.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/components/SubmitEvidence/SubmitEvidenceForm.tsx (1)
218-231: Trigger the onSuccess prop after successful tx.Parent wrappers can’t react (close modal/refresh) without this.
const onSuccessCallback = async () => { setLoading(false); // Generate a mock evidence ID - in real implementation, this would come from the transaction const evidenceId = `evidence_${Date.now()}`; setSubmittedEvidenceId(evidenceId); setIsSuccess(true); queueNotification({ header: 'Evidence Submitted Successfully!', - message: `Your evidence with file "${file?.name}" has been submitted for ${fellow.rank} rank.`, + message: `Your evidence with file "${file?.name}" has been submitted for ${fellow.rank} rank.`, status: 'success' }); setTxStatus(''); + onSuccess?.(); };
🧹 Nitpick comments (3)
src/components/SubmitEvidence/SubmitEvidenceForm.tsx (3)
393-399: Don’t override Controller’s ref with undefined.This can break focus-on-error and RHF internals. Remove the explicit ref.
<MarkdownEditor {...field} disabled={loading} - ref={undefined} />
292-301: Reset form state when submitting another evidence.Clear fields and file input so the form is fresh.
<Button variant='light' onPress={() => { setIsSuccess(false); setSubmittedEvidenceId(null); + setValue('evidence', ''); + setValue('file', null); + if (fileInputRef.current) fileInputRef.current.value = ''; + setError(''); + setTxStatus(''); }} className='text-text_secondary' >
371-378: File validation UI isn’t wired to RHF errors.
errors.filewill never populate since the file input isn’t registered. Either register the file field with RHF and add rules, or drop this block and rely onsetError(...)alerts.Would you like a patch to register the file input with
register('file')and movevalidateFormDatarules into RHF?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/SubmitEvidence/SubmitEvidenceForm.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/SubmitEvidence/SubmitEvidenceForm.tsx (2)
src/contexts/index.tsx (2)
useApiContext(28-28)useUserDetailsContext(28-28)src/utils/getSubstrateAddress.ts (1)
getSubstrateAddress(13-21)
🔇 Additional comments (2)
src/components/SubmitEvidence/SubmitEvidenceForm.tsx (2)
198-207: Confirm Wish enum argument shape for submitEvidence.Passing
'Promotion'as a string may depend on current metadata; some chains require a variant object (e.g.,{ Promotion: null }) or an explicit type.Example alternatives if needed:
// If variant object is required: const wishValue = { Promotion: null } as const; // Or construct explicitly with api.createType: const wishValue = api.createType('Wish', 'Promotion');Please verify against your chain’s metadata (polkadot-js types) and adjust accordingly.
308-435: Overall form flow and defensive checks look solid.Good early‑returns, status handling, and user feedback.
Ensure this form integrates with the modal/page wrappers as intended after the onSuccess fix.
| const selectedFile = watch('file'); | ||
|
|
||
| // Form validation state | ||
| const isFormValid = Boolean(selectedWallet && selectedAddress && api && apiReady && !loading); | ||
|
|
||
| // Notify parent component of form state changes | ||
| React.useEffect(() => { | ||
| onFormStateChange?.(isFormValid, loading); | ||
| }, [isFormValid, loading, onFormStateChange]); | ||
|
|
There was a problem hiding this comment.
Broaden isFormValid to include file and evidence validity.
Currently parents may enable submit while content is invalid.
- const selectedFile = watch('file');
+ const selectedFile = watch('file');
+ const evidenceValue = watch('evidence') ?? '';
+ const evidenceTrimmed = evidenceValue.trim();
+ const hasValidEvidence = evidenceTrimmed.length >= 10 && evidenceTrimmed.length <= 10000;
+ const hasFile = Boolean(selectedFile);
// Form validation state
- const isFormValid = Boolean(selectedWallet && selectedAddress && api && apiReady && !loading);
+ const isFormValid = Boolean(
+ selectedWallet &&
+ selectedAddress &&
+ api &&
+ apiReady &&
+ hasFile &&
+ hasValidEvidence &&
+ !loading
+ );📝 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.
| const selectedFile = watch('file'); | |
| // Form validation state | |
| const isFormValid = Boolean(selectedWallet && selectedAddress && api && apiReady && !loading); | |
| // Notify parent component of form state changes | |
| React.useEffect(() => { | |
| onFormStateChange?.(isFormValid, loading); | |
| }, [isFormValid, loading, onFormStateChange]); | |
| const selectedFile = watch('file'); | |
| const evidenceValue = watch('evidence') ?? ''; | |
| const evidenceTrimmed = evidenceValue.trim(); | |
| const hasValidEvidence = evidenceTrimmed.length >= 10 && evidenceTrimmed.length <= 10000; | |
| const hasFile = Boolean(selectedFile); | |
| // Form validation state | |
| const isFormValid = Boolean( | |
| selectedWallet && | |
| selectedAddress && | |
| api && | |
| apiReady && | |
| hasFile && | |
| hasValidEvidence && | |
| !loading | |
| ); | |
| // Notify parent component of form state changes | |
| React.useEffect(() => { | |
| onFormStateChange?.(isFormValid, loading); | |
| }, [isFormValid, loading, onFormStateChange]); |
🤖 Prompt for AI Agents
In src/components/SubmitEvidence/SubmitEvidenceForm.tsx around lines 60 to 69,
broaden isFormValid so parents can't enable submit when uploaded file or
evidence fields are invalid: include checks for selectedFile (truthy and valid
type/size) and the form's internal validation state (e.g. formState.isValid or
explicit checks for required evidence fields like title/description) when
computing isFormValid, update the React.useEffect dependency array to include
those new values, and ensure the file validation logic (type/size) is
implemented or referenced so isFormValid only becomes true when file and
evidence content are valid.
| const fellowAddress = getSubstrateAddress(selectedAddress.address); | ||
| if (!fellowAddress) { | ||
| setError('Invalid address format. Please select a valid address.'); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Normalize addresses when checking fellow membership.
Direct equality can fail across SS58 prefixes/0x. Normalize both sides.
- const fellowAddress = getSubstrateAddress(selectedAddress.address);
+ const fellowAddress = getSubstrateAddress(selectedAddress.address);
if (!fellowAddress) {
setError('Invalid address format. Please select a valid address.');
return;
}
...
- const fellow = fellows.find((f) => f.address === fellowAddress);
+ const fellow = fellows.find((f) => getSubstrateAddress(f.address) === fellowAddress);Also applies to: 177-181
🤖 Prompt for AI Agents
In src/components/SubmitEvidence/SubmitEvidenceForm.tsx around lines 165-169
(and similarly 177-181), the code compares addresses directly which can fail due
to differing SS58 prefixes or 0x hex vs SS58 formats; normalize both the
selected address and the fellow/member addresses to a common format before
comparing (e.g., convert both to a canonical substrate address via
getSubstrateAddress or a shared normalizeAddress helper, and compare the
resulting normalized strings, using consistent casing) so membership checks
succeed regardless of input address encoding.
1 participant
Summary by CodeRabbit
New Features
UI/Style
Removals
Chores