Skip to content

Conversation

@sirpy
Copy link
Contributor

@sirpy sirpy commented Feb 12, 2026

Description

#314

Summary by Sourcery

Enable bulk member management in the app and update development Celo deployment configuration and contracts.

New Features:

  • Add bulk member addition UI and logic allowing comma- or newline-separated wallet addresses to be added in one transaction for UBI and Direct Payments pools.
  • Expose success messaging and parsed address counts in the member management flow for clearer user feedback.
  • Introduce shared validation utilities for pool metadata and Direct Payments pool settings.

Enhancements:

  • Memoize the GoodCollective SDK instance in member management hooks to avoid redundant instantiation and reuse it across add/remove operations.

Build:

  • Switch the development Celo Hardhat RPC endpoint to Ankr and refresh deployed contract artifacts and release deployment metadata for that network.

@sirpy sirpy linked an issue Feb 12, 2026 that may be closed by this pull request
Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 2 issues, and left some high level feedback:

  • In validatePoolSettings, UBIPoolSettings is referenced but never imported/defined and settings.validEvents should likely be directPaymentSettings.validEvents, which will currently cause type or runtime errors.
  • The poolType discriminator in validatePoolSettings uses the string 'DirectPayments', which is inconsistent with the 'DIRECT'/'UBI' values used elsewhere (e.g., in useMemberManagement), so this branch may never execute as intended.
  • In ManageCollectivePage, the TextArea placeholder uses the HTML entity 
 for a newline; in this JSX/NativeBase context you likely want an actual newline ("\n" or {"\n"}) to avoid rendering the literal entity text.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `validatePoolSettings`, `UBIPoolSettings` is referenced but never imported/defined and `settings.validEvents` should likely be `directPaymentSettings.validEvents`, which will currently cause type or runtime errors.
- The `poolType` discriminator in `validatePoolSettings` uses the string `'DirectPayments'`, which is inconsistent with the `'DIRECT'`/`'UBI'` values used elsewhere (e.g., in `useMemberManagement`), so this branch may never execute as intended.
- In `ManageCollectivePage`, the `TextArea` placeholder uses the HTML entity `
` for a newline; in this JSX/NativeBase context you likely want an actual newline (`"\n"` or `{"\n"}`) to avoid rendering the literal entity text.

## Individual Comments

### Comment 1
<location> `packages/app/src/utils/validation.ts:37` </location>
<code_context>
+    if (directPaymentSettings.validEvents.length === 0) {
+      errors.validEvents = 'At least one valid event type is required';
+    }
+    if (directPaymentSettings.rewardPerEvent.length !== settings.validEvents.length) {
+      errors.rewardPerEvent = 'Must specify reward amount for each event type';
+    }
</code_context>

<issue_to_address>
**issue (bug_risk):** The reference to `settings.validEvents` is likely a bug and should use `directPaymentSettings.validEvents` instead.

In `validatePoolSettings`, `settings` is not in scope, so this line will fail at type-check or runtime. This should compare `rewardPerEvent.length` to `directPaymentSettings.validEvents.length` instead.
</issue_to_address>

### Comment 2
<location> `packages/app/src/pages/ManageCollectivePage.tsx:426` </location>
<code_context>
-                  </HStack>
+                  <Text fontWeight="600">Wallet Addresses</Text>
+                  <TextArea
+                    placeholder="0xabc...123, 0xdef...456&#10;0xghi...789"
+                    value={memberManagement.memberInput}
+                    onChangeText={memberManagement.setMemberInput}
</code_context>

<issue_to_address>
**issue (bug_risk):** The use of `&#10;` in the placeholder likely won’t render as a newline in React Native and may show the raw entity.

Because React Native doesn’t parse HTML entities in JSX strings, this will likely render the literal `&#10;` instead of a newline. To get a line break in the placeholder, use an actual newline in a template string or `{"\n"}` between the examples (e.g., `"0xabc...123, 0xdef...456\n0xghi...789"`).
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

if (directPaymentSettings.validEvents.length === 0) {
errors.validEvents = 'At least one valid event type is required';
}
if (directPaymentSettings.rewardPerEvent.length !== settings.validEvents.length) {
Copy link

Choose a reason for hiding this comment

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

issue (bug_risk): The reference to settings.validEvents is likely a bug and should use directPaymentSettings.validEvents instead.

In validatePoolSettings, settings is not in scope, so this line will fail at type-check or runtime. This should compare rewardPerEvent.length to directPaymentSettings.validEvents.length instead.

</HStack>
<Text fontWeight="600">Wallet Addresses</Text>
<TextArea
placeholder="0xabc...123, 0xdef...456&#10;0xghi...789"
Copy link

Choose a reason for hiding this comment

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

issue (bug_risk): The use of &#10; in the placeholder likely won’t render as a newline in React Native and may show the raw entity.

Because React Native doesn’t parse HTML entities in JSX strings, this will likely render the literal &#10; instead of a newline. To get a line break in the placeholder, use an actual newline in a template string or {"\n"} between the examples (e.g., "0xabc...123, 0xdef...456\n0xghi...789").

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature: Bulk Add Members to Pools in UI

1 participant