Feature/boe v2 - #100
Open
manishdex25 wants to merge 8 commits into
Open
Conversation
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/common/Navbar/Navbar.tsx (1)
299-322: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClose the mobile menu after Partners navigation.
The new
Partnerslink does not resetisMobileMenuOpen. The mobile overlay remains open after navigation and covers the Partners page. Add the same close handler used by the Settings and Contact links. Add a test that opens the mobile menu, selects Partners, and verifies that the menu closes.Proposed fix
<Link to="/partners" + onClick={() => setIsMobileMenuOpen(false)} className="px-4 py-3 text-left text-sm font-bold font-urbanist rounded-lg transition-colors duration-200"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/common/Navbar/Navbar.tsx` around lines 299 - 322, Update the Partners Link in the mobile navigation to use the same isMobileMenuOpen reset handler as the Settings and Contact links, so navigation closes the overlay. Add a test covering opening the mobile menu, selecting Partners, and verifying the menu is closed.src/components/AssetManagementPanel/AssetManagementForm/AssetManagementDropdown/AssetManagementDropdown.tsx (1)
154-176: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the document type consistent through the return flow.
If
documentLabelisBoE, these menu items use BoE copy. The selected return forms and return overlays still contain hard-codedETRtext. Pass the document label through those views, or use neutral copy. This prevents a BoE action from being labelled as an ETR action after selection.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/AssetManagementPanel/AssetManagementForm/AssetManagementDropdown/AssetManagementDropdown.tsx` around lines 154 - 176, Propagate documentLabel from AssetManagementDropdown through the AcceptReturnToIssuer and RejectReturnToIssuer form and overlay views, replacing hard-coded ETR text with the selected label. Ensure the entire return flow preserves BoE or ETR consistently after an action is selected.
🧹 Nitpick comments (1)
src/hooks/useContractFunctionHook.obligation.test.tsx (1)
127-153: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest
rejectanddischargerouting.This suite tests
accept, but the new obligation map also exposesrejectanddischarge. Add asendtest for each operation. Assert the mapped registry options,keyId, target function, and confirmation state. (raw.githubusercontent.com)Based on the supplied obligation method map, these are independent lifecycle routes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/useContractFunctionHook.obligation.test.tsx` around lines 127 - 153, Extend the obligation-routing tests alongside the existing accept case to cover send calls for reject and discharge. In each test, configure useDocumentContext with the obligation keyId, mock the corresponding registry method, invoke useContractFunctionHook.send, and assert the mapped registry options, keyId, target function, and final CONFIRMED state.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/components/AssetManagementPanel/AssetManagementForm/AssetManagementDropdown/AssetManagementDropdown.tsx`:
- Around line 61-94: The lifecycle actions in AssetManagementDropdown are
rendered through non-focusable DropdownItem elements, so make these controls
keyboard-operable by using semantic button elements or enhancing DropdownItem
with focusable menu-item semantics and Enter/Space activation. Preserve the
existing canAcceptObligation, canRejectObligation, canDischargeObligation guards
and onSetFormAction mappings.
In
`@src/components/common/contexts/TokenInformationContext/TokenInformationContext.tsx`:
- Around line 329-363: Update TokenInformationContext’s acceptObligation,
rejectObligation, and dischargeObligation write hooks to use the document-chain
provider and reject requests unless the signer’s normalized chain matches the
document chain. In VerifyResult.tsx at lines 77-85, normalize both chain values
with toChainId and render BoE asset actions only after the network switch
succeeds and currentChainId matches the document chain.
In `@src/components/home/VerifySection/VerifySection.tsx`:
- Line 184: Update the VerifySection call to VerifyResult so BoE obligations are
not marked transferable; pass isObligation separately as the obligation
indicator. In VerifyResult, use that indicator to bypass delegation checks,
stored-paymaster checks, and paymaster-card rendering, preserving gasless
behavior only for non-obligation transfers.
In `@src/hooks/useContractFunctionHook.tsx`:
- Around line 198-203: Update the error-message handling in
useContractFunctionHook around the unsupported-method error and ordinary
contract rejections so unrecognized errors fall back to Error.message when
getMetaMaskErrorMessage returns an empty string. Reuse the fallback behavior
established by makeGaslessHook, preserving existing MetaMask-specific messages.
---
Outside diff comments:
In
`@src/components/AssetManagementPanel/AssetManagementForm/AssetManagementDropdown/AssetManagementDropdown.tsx`:
- Around line 154-176: Propagate documentLabel from AssetManagementDropdown
through the AcceptReturnToIssuer and RejectReturnToIssuer form and overlay
views, replacing hard-coded ETR text with the selected label. Ensure the entire
return flow preserves BoE or ETR consistently after an action is selected.
In `@src/components/common/Navbar/Navbar.tsx`:
- Around line 299-322: Update the Partners Link in the mobile navigation to use
the same isMobileMenuOpen reset handler as the Settings and Contact links, so
navigation closes the overlay. Add a test covering opening the mobile menu,
selecting Partners, and verifying the menu is closed.
---
Nitpick comments:
In `@src/hooks/useContractFunctionHook.obligation.test.tsx`:
- Around line 127-153: Extend the obligation-routing tests alongside the
existing accept case to cover send calls for reject and discharge. In each test,
configure useDocumentContext with the obligation keyId, mock the corresponding
registry method, invoke useContractFunctionHook.send, and assert the mapped
registry options, keyId, target function, and final CONFIRMED state.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5004c5ad-ca44-4b51-a1a6-b0f7cde7ec12
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (49)
package.jsonsrc/App.test.tsxsrc/__tests__/home.test.tsxsrc/components/AssetManagementPanel/AssetManagementActions/index.tsxsrc/components/AssetManagementPanel/AssetManagementApplication/index.tsxsrc/components/AssetManagementPanel/AssetManagementForm/AssetManagementDropdown/AssetManagementDropdown.tsxsrc/components/AssetManagementPanel/AssetManagementForm/AssetManagementForm.test.tsxsrc/components/AssetManagementPanel/AssetManagementForm/AssetManagementForm.tsxsrc/components/AssetManagementPanel/AssetManagementForm/FormVariants/ActionForm/ActionForm.tsxsrc/components/AssetManagementPanel/AssetManagementForm/FormVariants/ActionForm/types.tssrc/components/AssetManagementPanel/AssetManagementForm/FormVariants/ActionSelectionForm/ActionSelectionForm.obligation.test.tsxsrc/components/AssetManagementPanel/AssetManagementForm/FormVariants/ActionSelectionForm/ActionSelectionForm.test.tsxsrc/components/AssetManagementPanel/AssetManagementForm/FormVariants/ActionSelectionForm/ActionSelectionForm.tsxsrc/components/common/Navbar/Navbar.test.tsxsrc/components/common/Navbar/Navbar.tsxsrc/components/common/Overlay/OverlayContent/DocumentTransferMessage.tsxsrc/components/common/contexts/DocumentContext/DocumentContext.tsxsrc/components/common/contexts/TokenInformationContext/TokenInformationContext.tsxsrc/components/common/contexts/providerContext.tsxsrc/components/home/EndorsementChain/useEndorsementChain.test.tssrc/components/home/EndorsementChain/useEndorsementChain.tssrc/components/home/VerifySection/VerifyResult.tsxsrc/components/home/VerifySection/VerifySection.test.tsxsrc/components/home/VerifySection/VerifySection.tsxsrc/components/home/VerifySection/useVerify.test.tssrc/components/home/VerifySection/useVerify.tssrc/constants/index.tssrc/constants/obligation.tssrc/gasless/gaslessHooks.test.tsxsrc/gasless/makeGaslessHook.tssrc/gasless/useGaslessAcceptReturned.tssrc/gasless/useGaslessNominate.tssrc/gasless/useGaslessRejectReturned.tssrc/gasless/useGaslessRejectTransferBeneficiary.tssrc/gasless/useGaslessRejectTransferHolder.tssrc/gasless/useGaslessRejectTransferOwners.tssrc/gasless/useGaslessReturnToIssuer.tssrc/gasless/useGaslessTransferBeneficiary.tssrc/gasless/useGaslessTransferHolder.tssrc/gasless/useGaslessTransferOwners.tssrc/hooks/useContractFunctionHook.obligation.test.tsxsrc/hooks/useContractFunctionHook.test.tsxsrc/hooks/useContractFunctionHook.tsxsrc/hooks/useIsObligation.tssrc/hooks/useTitleEscrowContract.tssrc/hooks/useTokenRegistryContract.tssrc/pages/Home/index.tsxsrc/routes.tsxsrc/utils/chain-utils.ts
- Added isObligation prop to DocumentStatus and AssetManagementTags components to manage obligation-specific UI behavior. - Enhanced ActionSelectionForm to display obligation status labels and manage visibility based on isObligation state.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/index.css (1)
4077-4102: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the Stylelint errors.
Stylelint reports empty lines before declarations at Lines 4077, 4097, and 4102. It also reports quoted
Urbanistfont names at Lines 4109 and 4129. Remove the empty lines and quotes to satisfy the configured stylesheet rules.Proposed fix
- font-family: 'Urbanist'; + font-family: Urbanist;Also applies to: 4109-4129
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/index.css` around lines 4077 - 4102, Update the CSS declarations around the dropdown item styles to remove the reported empty lines before declarations at the referenced locations, and use unquoted Urbanist font names wherever they appear in the affected styles. Preserve all other styling and formatting behavior.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/index.css`:
- Around line 4077-4102: Update the CSS declarations around the dropdown item
styles to remove the reported empty lines before declarations at the referenced
locations, and use unquoted Urbanist font names wherever they appear in the
affected styles. Preserve all other styling and formatting behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: aac3827e-faf7-44ea-b21e-6a6253f90ba1
📒 Files selected for processing (3)
src/components/AssetManagementPanel/AssetManagementForm/AssetManagementDropdown/AssetManagementDropdown.tsxsrc/components/Dropdown/Dropdown.tsxsrc/index.css
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/AssetManagementPanel/AssetManagementForm/AssetManagementDropdown/AssetManagementDropdown.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes