-
Notifications
You must be signed in to change notification settings - Fork 177
Feature/candidate onboarding terms #213
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Feature/candidate onboarding terms #213
Conversation
📝 WalkthroughWalkthroughMinor formatting in ConnectWallet; CreateProcessPage adds validation, error handling, pending UI and success modal around deployment; AddCandidateModal requires explicit Terms & Conditions acceptance before confirming a candidate. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as CreateProcessPage
participant Deploy as Blockchain/DeployService
participant Modal as SuccessModal
User->>UI: Click "Create new process"
rect `#E8F0FE`
Note over UI: Validate form (proposals ≥ 2)
UI->>UI: If invalid -> show validation message / stop
end
rect `#FFF4E6`
Note over UI: Prepare & Pending
UI->>UI: Format proposals (UTF-8 bytes)
UI->>UI: Set pending = true (disable submit, show spinner)
end
rect `#E8F8EE`
Note over UI,Deploy: Deployment attempt
UI->>Deploy: Deploy voting process transaction
alt success
Deploy-->>UI: Transaction result
UI->>Modal: Open success modal with tx details
Modal-->>User: Display transaction details
else failure
Deploy-->>UI: Error
UI-->>User: Alert "Transaction failed"
end
end
UI->>UI: Clear pending state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
anonymousVoting/clientAnonymousVoting/src/components/modals/AddCandidateModal.js (2)
39-54: Consider validating candidate name before submission.While the Terms & Conditions validation is properly implemented, there's no validation for the candidate name field. An empty or whitespace-only name could be submitted to the blockchain.
🔎 Suggested validation
// ✅ ADDED: enforce terms acceptance if (!acceptedTerms) { toast.error("You must accept the Terms & Conditions"); return; } +if (!candidateDetail.name.trim()) { + toast.error("Candidate name is required"); + return; +} + try {
47-47: Uncomment or remove the success toast notification.The success toast notification is commented out. Consider either uncommenting it to provide user feedback on successful candidate addition, or removing it if it's no longer needed.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
anonymousVoting/clientAnonymousVoting/src/components/ConnectWallet.jsanonymousVoting/clientAnonymousVoting/src/components/CreateProcessPage.jsanonymousVoting/clientAnonymousVoting/src/components/modals/AddCandidateModal.js
🔇 Additional comments (10)
anonymousVoting/clientAnonymousVoting/src/components/modals/AddCandidateModal.js (5)
17-18: LGTM: Terms acceptance state properly initialized.The
acceptedTermsstate is correctly initialized tofalse, ensuring users must explicitly accept terms before submission.
32-36: LGTM: Terms acceptance validation correctly blocks submission.The early return prevents candidate creation if terms are not accepted, and displays an appropriate error message to the user.
122-132: LGTM: Terms & Conditions checkbox properly implemented.The checkbox is correctly bound to the
acceptedTermsstate and provides clear user feedback about the requirement.
138-145: LGTM: Confirm button properly disabled based on terms acceptance.The button's disabled state correctly enforces the Terms & Conditions acceptance requirement, preventing submission until the checkbox is checked.
56-60: LGTM: Terms acceptance state properly reset on modal close.Resetting
acceptedTermswhen closing the modal ensures a fresh state for the next candidate addition, preventing accidental carry-over of the previous acceptance.anonymousVoting/clientAnonymousVoting/src/components/CreateProcessPage.js (5)
8-9: LGTM: Cleaned up unused imports.Removing unused imports improves code maintainability and reduces bundle size.
35-38: LGTM: Early form validation prevents unnecessary processing.The early validation check properly prevents the deployment process from starting when the form is invalid, providing immediate feedback to the user.
40-60: LGTM: Robust error handling with try/catch/finally.The error handling structure properly:
- Sets pending state before deployment
- Captures and logs errors
- Alerts users on failure
- Ensures pending state is reset in the finally block regardless of success or failure
114-131: LGTM: Loading state provides excellent user feedback.The implementation properly:
- Disables the button during pending state to prevent double-submission
- Shows a spinner with "Creating..." text during the transaction
- Provides clear visual feedback to the user
64-66: Minimum proposal count requirement is correctly aligned.The client-side validation requiring at least 2 proposals (via
proposals.split(',').length >= 2) matches the smart contract requirement in ElectionFactory.sol line 63, which enforces_candidates.length<2revert. Both enforce the same minimum threshold and align with the business logic.
anonymousVoting/clientAnonymousVoting/src/components/ConnectWallet.js
Outdated
Show resolved
Hide resolved
anonymousVoting/clientAnonymousVoting/src/components/modals/AddCandidateModal.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
anonymousVoting/clientAnonymousVoting/src/components/modals/AddCandidateModal.js (1)
43-46: Add success feedback after candidate is added.After the transaction completes successfully (line 43), the modal closes silently. Users receive no confirmation that the candidate was added. The
successtoastfunction is imported on line 4 but never used.Consider adding a success toast before closing the modal to improve user experience.
🔎 Proposed enhancement
await tx.wait(); + toast.success("Candidate added successfully"); setIsOpen(false); setAcceptedTerms(false);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
anonymousVoting/clientAnonymousVoting/src/components/modals/AddCandidateModal.js
🧰 Additional context used
🧬 Code graph analysis (1)
anonymousVoting/clientAnonymousVoting/src/components/modals/AddCandidateModal.js (1)
anonymousVoting/clientAnonymousVoting/src/components/CreateProcessPage.js (1)
name(17-17)
🔇 Additional comments (1)
anonymousVoting/clientAnonymousVoting/src/components/modals/AddCandidateModal.js (1)
16-17: Clarify whether this implementation fully addresses the intended requirements for candidate verification.The current implementation adds a Terms & Conditions acceptance requirement that the admin/creator must check before adding a candidate to the election. However, if the intended requirement is to ensure that the candidate themselves (the person being added) verifies their participation—including aspects like sending verification links, tracking pending acceptance states, and preventing campaign activation until candidate confirmation—then this implementation does not address those needs.
Please confirm:
- Whether this PR is intended to fully address the candidate verification/acceptance requirement, or if this is only a partial first step
- What the actual requirement specifies regarding who must accept and what the full verification flow should be
- Whether additional implementation (candidate notification, verification mechanism, state tracking) is needed
If full candidate self-acceptance/verification is required, the current approach of admin acceptance before creation would need to be combined with a post-creation candidate acceptance mechanism.
anonymousVoting/clientAnonymousVoting/src/components/modals/AddCandidateModal.js
Outdated
Show resolved
Hide resolved
anonymousVoting/clientAnonymousVoting/src/components/modals/AddCandidateModal.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
anonymousVoting/clientAnonymousVoting/src/components/modals/AddCandidateModal.js (1)
30-39: Validate that candidate name is not empty (previously flagged but still missing).The code checks for terms acceptance but doesn't validate that
candidateDetail.name.trim()is non-empty before callingaddProposal. A user can accept terms and submit with whitespace-only input, creating a candidate with an empty name.This issue was marked as addressed in a previous review but the validation is not present in the current code.
🔎 Proposed fix
if (!acceptedTerms) { toast.error("You must accept the Terms & Conditions"); return; } + + const trimmedName = candidateDetail.name.trim(); + if (!trimmedName) { + toast.error("Candidate name cannot be empty"); + return; + } try { let tx = await addProposal( electionId, - ethers.utils.toUtf8Bytes(candidateDetail.name.trim()) + ethers.utils.toUtf8Bytes(trimmedName) );
🧹 Nitpick comments (1)
anonymousVoting/clientAnonymousVoting/src/components/modals/AddCandidateModal.js (1)
67-69: Consider using a button element for better accessibility.The current implementation uses a
divwithonClick, which is not keyboard-accessible by default. Users navigating via keyboard cannot focus or activate this trigger.🔎 Proposed fix
- <div onClick={openModal} style={{ cursor: "pointer" }}> + <button + onClick={openModal} + style={{ + cursor: "pointer", + background: "none", + border: "none", + padding: 0, + font: "inherit", + color: "inherit" + }} + > <font size="2">Add Candidate</font> - </div> + </button>
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
anonymousVoting/clientAnonymousVoting/src/components/modals/AddCandidateModal.js
🔇 Additional comments (2)
anonymousVoting/clientAnonymousVoting/src/components/modals/AddCandidateModal.js (2)
105-116: Terms & Conditions checkbox implementation looks good.The checkbox is properly bound to the
acceptedTermsstate, and the user interaction is handled correctly. The styling and accessibility of the label are appropriate.
122-129: Button disabled logic is correctly implemented.The Confirm button is appropriately disabled when terms are not accepted, providing clear visual feedback to users and preventing invalid submissions.
Description
This PR introduces a mandatory Terms & Conditions acceptance step during candidate onboarding.
Previously, candidates could be added to an election without explicitly accepting any terms. This update enforces explicit consent at the UI level by requiring candidates to accept the Terms & Conditions before they can be successfully added.
Changes Made
Added a Terms & Conditions checkbox to AddCandidateModal
Disabled the Confirm button until terms are accepted
Prevented candidate creation if terms are not accepted
No changes to existing smart contract calls or business logic
Scope
UI-only change
No backend or smart contract modifications
Minimal and isolated to the candidate onboarding flow
Testing
Verified that a candidate cannot be added without accepting terms
Verified normal candidate creation after accepting terms
Fixes
Fixes #190
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.