feat: tranfer token ownership #152
Conversation
|
Warning Rate limit exceeded@fmorency has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 24 minutes and 5 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe pull request introduces enhancements to the token management functionality within the application. Key changes include the addition of a Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #152 +/- ##
==========================================
+ Coverage 57.01% 57.39% +0.38%
==========================================
Files 145 147 +2
Lines 14436 14706 +270
==========================================
+ Hits 8230 8440 +210
- Misses 6206 6266 +60 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (13)
components/factory/modals/TransferModal.tsx (5)
107-122: Consider extracting inline styles to a CSS class or styled componentThe inline styles applied to the
<dialog>component make the code less maintainable and harder to read. Extracting these styles into a CSS class or using a styled component can improve readability and facilitate reusability.
128-129: Remove redundant Formik validation propsThe props
validateOnChangeandvalidateOnBluraretrueby default in Formik. Explicitly setting them is unnecessary unless you intend to override default behavior.Apply this diff to simplify the code:
initialValues={formData} validationSchema={TokenOwnershipSchema} onSubmit={(values, { resetForm }) => handleTransfer(values, resetForm)} - validateOnChange={true} - validateOnBlur={true}
137-137: Simplify theonClickhandler for the close buttonYou can pass
resetFormdirectly tohandleCloseModalwithout wrapping it in an additional arrow function, simplifying the code.Apply this diff to streamline the function call:
- onClick={() => handleCloseModal(() => resetForm())} + onClick={() => handleCloseModal(resetForm)}
36-44: Add unit tests for the Escape key handling inuseEffectThe
useEffecthook manages the modal's visibility based on the Escape key press. Consider adding unit tests to ensure this behavior functions as expected across different scenarios.Would you like help in creating these unit tests?
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 36-38: components/factory/modals/TransferModal.tsx#L36-L38
Added lines #L36 - L38 were not covered by tests
47-50: EnsurehandleCloseModalproperly resets the formWhile the
handleCloseModalfunction closes the modal and optionally resets the form, adding tests can verify that the form state is correctly managed when the modal is closed in various ways.Would you like assistance in writing tests for this functionality?
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 47-48: components/factory/modals/TransferModal.tsx#L47-L48
Added lines #L47 - L48 were not covered by testshooks/useOsmosisRpcQueryClient.ts (1)
12-14: Add unit tests foruseOsmosisRpcQueryClientAdding unit tests for this hook will help ensure it behaves correctly when
selectedEndpoint?.rpcis undefined, null, or an invalid URL, enhancing the robustness of the application.Would you like assistance in writing these unit tests?
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 12-14: hooks/useOsmosisRpcQueryClient.ts#L12-L14
Added lines #L12 - L14 were not covered by testscomponents/factory/components/MyDenoms.tsx (2)
75-86: Add test coverage for transfer functionality.The new transfer-related code paths are not covered by tests. This includes:
- Modal type handling
- Transfer action handling
- Transfer modal state management
Would you like me to help generate test cases for these scenarios:
- Transfer modal opening/closing
- Transfer action handling
- Router query updates
- Modal state management
Also applies to: 122-129
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 75-75: components/factory/components/MyDenoms.tsx#L75
Added line #L75 was not covered by tests
[warning] 78-80: components/factory/components/MyDenoms.tsx#L78-L80
Added lines #L78 - L80 were not covered by tests
[warning] 84-86: components/factory/components/MyDenoms.tsx#L84-L86
Added lines #L84 - L86 were not covered by tests
397-415: Consider consolidating modal state management.The TransferModal implementation follows the same pattern as other modals but introduces duplicate state management logic. Consider extracting common modal behavior into a custom hook.
Example implementation:
const useModalState = (modalType: string, onSuccess?: () => void) => { const [isOpen, setIsOpen] = useState(false); const handleClose = () => { setIsOpen(false); handleCloseModal(); }; const handleSuccess = () => { onSuccess?.(); handleUpdateModalClose(); }; return { isOpen, setIsOpen, handleClose, handleSuccess }; };🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 400-403: components/factory/components/MyDenoms.tsx#L400-L403
Added lines #L400 - L403 were not covered by tests
[warning] 407-408: components/factory/components/MyDenoms.tsx#L407-L408
Added lines #L407 - L408 were not covered by testshooks/useQueries.ts (3)
Line range hint
583-613: LGTM! The rename improves clarity and the refetch configuration is appropriate.The function rename from
useTokenFactoryDenomstouseTokenFactoryDenomsFromAdminbetter reflects its purpose, and the frequent refetching configuration is appropriate for tracking ownership changes.Consider adding unit tests to cover this hook, as indicated by the code coverage report.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 593-594: hooks/useQueries.ts#L593-L594
Added lines #L593 - L594 were not covered by tests
615-645: LGTM! Well-structured hook with proper error handling.The implementation is solid with:
- Well-justified use of RPC client (as commented)
- Comprehensive error handling
- Consistent refetch configuration with
useTokenFactoryDenomsFromAdminConsider adding unit tests to cover this hook, as indicated by the code coverage report. I can help generate test cases if needed.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 619-627: hooks/useQueries.ts#L619-L627
Added lines #L619 - L627 were not covered by tests
Line range hint
583-645: Offer assistance with test coverageBoth new hooks are critical for token ownership transfer functionality but lack test coverage. Would you like me to help generate comprehensive test cases for:
useTokenFactoryDenomsFromAdminuseDenomAuthorityMetadataThe test suite should cover:
- Successful queries
- Error handling scenarios
- Refetch behavior
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 619-627: hooks/useQueries.ts#L619-L627
Added lines #L619 - L627 were not covered by testspages/factory/index.tsx (2)
Line range hint
39-63: Consider extracting token processing logicWhile the implementation is correct, the data processing logic could be more maintainable by extracting it into a separate utility function.
Consider this refactor:
+ const processTokenData = ( + denom: string, + metadatas: MetadataSDKType[], + balances: Balance[], + totalSupply: Supply[] + ): ExtendedMetadataSDKType | null => { + const metadata = metadatas.find(meta => meta.base === denom); + const balance = balances.find(bal => bal.denom === denom); + const supply = totalSupply.find(supply => supply.denom === denom); + return metadata + ? { + ...metadata, + balance: balance?.amount || '0', + totalSupply: supply?.amount || '0', + } + : null; + }; const combinedData = useMemo(() => { if (denoms?.denoms && metadatas?.metadatas && balances && totalSupply) { const otherTokens = denoms.denoms .filter(denom => denom !== 'umfx') - .map((denom: string) => { - const metadata = metadatas.metadatas.find(meta => meta.base === denom); - const balance = balances.find(bal => bal.denom === denom); - const supply = totalSupply.find(supply => supply.denom === denom); - return metadata - ? { - ...metadata, - balance: balance?.amount || '0', - totalSupply: supply?.amount || '0', - } - : null; - }) + .map((denom: string) => processTokenData(denom, metadatas.metadatas, balances, totalSupply)) .filter((meta): meta is ExtendedMetadataSDKType => meta !== null); return [...otherTokens]; } return []; }, [denoms, metadatas, balances, totalSupply]);
Line range hint
82-89: Update placeholder SEO metadata URLsThe SEO metadata contains placeholder URLs that should be updated with actual production URLs.
Consider updating these placeholders:
- <meta property="og:url" content="https://" /> - <meta property="og:image" content="https://" /> + <meta property="og:url" content="https://your-production-url.com/factory" /> + <meta property="og:image" content="https://your-production-url.com/og-image.png" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (8)
components/factory/components/MyDenoms.tsx(10 hunks)components/factory/modals/TransferModal.tsx(1 hunks)components/factory/modals/index.ts(1 hunks)helpers/formReducer.tsx(1 hunks)hooks/useOsmosisRpcQueryClient.ts(1 hunks)hooks/useQueries.ts(4 hunks)package.json(1 hunks)pages/factory/index.tsx(2 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
hooks/useOsmosisRpcQueryClient.ts
[warning] 12-14: hooks/useOsmosisRpcQueryClient.ts#L12-L14
Added lines #L12 - L14 were not covered by tests
components/factory/modals/TransferModal.tsx
[warning] 36-38: components/factory/modals/TransferModal.tsx#L36-L38
Added lines #L36 - L38 were not covered by tests
[warning] 47-48: components/factory/modals/TransferModal.tsx#L47-L48
Added lines #L47 - L48 were not covered by tests
[warning] 63-98: components/factory/modals/TransferModal.tsx#L63-L98
Added lines #L63 - L98 were not covered by tests
[warning] 221-221: components/factory/modals/TransferModal.tsx#L221
Added line #L221 was not covered by tests
hooks/useQueries.ts
[warning] 583-583: hooks/useQueries.ts#L583
Added line #L583 was not covered by tests
[warning] 593-594: hooks/useQueries.ts#L593-L594
Added lines #L593 - L594 were not covered by tests
[warning] 619-627: hooks/useQueries.ts#L619-L627
Added lines #L619 - L627 were not covered by tests
components/factory/components/MyDenoms.tsx
[warning] 75-75: components/factory/components/MyDenoms.tsx#L75
Added line #L75 was not covered by tests
[warning] 78-80: components/factory/components/MyDenoms.tsx#L78-L80
Added lines #L78 - L80 were not covered by tests
[warning] 84-86: components/factory/components/MyDenoms.tsx#L84-L86
Added lines #L84 - L86 were not covered by tests
[warning] 101-101: components/factory/components/MyDenoms.tsx#L101
Added line #L101 was not covered by tests
[warning] 108-108: components/factory/components/MyDenoms.tsx#L108
Added line #L108 was not covered by tests
[warning] 122-127: components/factory/components/MyDenoms.tsx#L122-L127
Added lines #L122 - L127 were not covered by tests
[warning] 400-403: components/factory/components/MyDenoms.tsx#L400-L403
Added lines #L400 - L403 were not covered by tests
[warning] 407-408: components/factory/components/MyDenoms.tsx#L407-L408
Added lines #L407 - L408 were not covered by tests
🔇 Additional comments (8)
components/factory/modals/index.ts (1)
5-5: Export statement for TransferModal is correctly added
The export statement for TransferModal is properly added and follows the existing naming conventions and structure.
hooks/useOsmosisRpcQueryClient.ts (1)
12-14:
Handle potential issues with empty rpcEndpoint
The rpcEndpoint defaults to an empty string if selectedEndpoint?.rpc is undefined. Verify that createRpcQueryClient can handle an empty string without throwing an error.
Apply this diff to prevent potential issues:
queryFn: () =>
createRpcQueryClient({
- rpcEndpoint: selectedEndpoint?.rpc || '',
+ rpcEndpoint: selectedEndpoint?.rpc ?? '',
}),This change ensures that undefined is passed instead of an empty string when the rpcEndpoint is not available.
Likely invalid or redundant comment.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 12-14: hooks/useOsmosisRpcQueryClient.ts#L12-L14
Added lines #L12 - L14 were not covered by tests
package.json (1)
42-42: Verify compatibility with updated dependency versions
Several dependencies have been updated:
@liftedinit/manifestjsto0.0.1-rc.1reactandreact-domto18.3.1@types/reactand@types/react-domto match React version
Ensure that these updates are compatible with your application and that all components function as expected. Pay special attention to any breaking changes or deprecated features in the new versions.
Also applies to: 66-67, 70-71
helpers/formReducer.tsx (1)
381-385: LGTM! Clean type definition.
The TransferTokenFormData type is well-structured with appropriate field types.
components/factory/components/MyDenoms.tsx (1)
491-498: LGTM! Clean implementation of transfer button.
The transfer button implementation:
- Correctly disables for 'umfx' tokens
- Follows consistent styling with other action buttons
- Properly handles event propagation
pages/factory/index.tsx (3)
20-22: LGTM! Well-structured hook implementation
The hook usage is well-implemented with proper error handling and null safety for the address parameter.
1-1: Verify the hook rename impact across the codebase
The hook rename from useTokenFactoryDenoms to useTokenFactoryDenomsFromAdmin better reflects its purpose for token ownership management. However, we should ensure all usages are updated.
Also applies to: 5-5
20-22: Verify token ownership filtering
The implementation aligns with the PR objectives to show only user-owned tokens. Let's verify the filtering logic in the related hooks.
Also applies to: 39-63
✅ Verification successful
Let me gather more information about the token factory queries and filtering logic.
Token ownership filtering is correctly implemented
The verification confirms that the token ownership filtering is properly implemented through the denomsFromAdmin query. The query specifically filters tokens by the admin address parameter, ensuring only tokens owned by the user are returned. The implementation in useTokenFactoryDenomsFromAdmin hook correctly passes the user's address and handles the data fetching with appropriate loading states.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check the implementation of useTokenFactoryDenomsFromAdmin
rg -A 10 "useTokenFactoryDenomsFromAdmin.*=.*"
# Check for any ownership-related queries
ast-grep --pattern 'query.*owner.*'
Length of output: 684
Script:
#!/bin/bash
# Check the complete implementation of the token factory query
rg -A 15 "denomsFromAdmin"
# Check the component implementation to understand how the data is filtered
rg -A 20 "const.*denoms.*useTokenFactoryDenomsFromAdmin"
Length of output: 2087
| const handleTransfer = async (values: FormikValues, resetForm: () => void) => { | ||
| setIsSigning(true); | ||
| try { | ||
| const msg = changeAdmin({ | ||
| sender: address, | ||
| denom: denom?.base ?? '', | ||
| newAdmin: values.newAdmin, | ||
| }); | ||
|
|
||
| const fee = await estimateFee(address, [msg]); | ||
|
|
||
| await tx([msg], { | ||
| fee, | ||
| onSuccess: () => { | ||
| onSuccess(); | ||
| handleCloseModal(resetForm); | ||
| }, | ||
| }); | ||
| } catch (error) { | ||
| console.error('Error during transaction setup:', error); | ||
| let errorMessage = 'An unknown error occurred while adding the endpoint.'; | ||
|
|
||
| if (error instanceof Error) { | ||
| if (error.message.includes('unauthorized account')) { | ||
| errorMessage = 'Unauthorized account. Please check your account and try again.'; | ||
| } | ||
| } | ||
|
|
||
| setToastMessage({ | ||
| type: 'alert-error', | ||
| title: 'Error transferring ownership', | ||
| description: errorMessage, | ||
| bgColor: '#e74c3c', | ||
| }); | ||
| throw error; | ||
| } finally { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add unit tests for the handleTransfer function
The handleTransfer function involves critical logic for transaction handling and error management. Adding unit tests will enhance reliability and ensure that any future changes do not break existing functionality.
Would you like assistance in writing unit tests for this function?
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 63-98: components/factory/modals/TransferModal.tsx#L63-L98
Added lines #L63 - L98 were not covered by tests
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Félix C. Morency <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
components/factory/components/MyDenoms.tsx (2)
38-38: Consider Using a Dedicated Type or Enum for Modal TypesTo enhance maintainability and reduce repetition, consider defining a dedicated type or enum for
modalType. This approach centralizes the modal types, making it easier to manage and less error-prone when adding or modifying modal actions.For example:
type ModalType = 'mint' | 'burn' | 'multimint' | 'multiburn' | 'update' | 'info' | 'transfer' | null; const [modalType, setModalType] = useState<ModalType>(null);This way, you can reuse
ModalTypethroughout your codebase, ensuring consistency and simplifying future updates.
Line range hint
38-413: Add Unit Tests for New Transfer FunctionalityThe new transfer functionality, including state variables and handlers, is not covered by unit tests. To ensure reliability and prevent regressions, please add tests covering these additions.
Would you like assistance in generating unit tests for the transfer functionality or creating a GitHub issue to track this task?
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 399-402: components/factory/components/MyDenoms.tsx#L399-L402
Added lines #L399 - L402 were not covered by tests
[warning] 406-407: components/factory/components/MyDenoms.tsx#L406-L407
Added lines #L406 - L407 were not covered by tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
components/factory/components/MyDenoms.tsx(9 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
components/factory/components/MyDenoms.tsx
[warning] 74-74: components/factory/components/MyDenoms.tsx#L74
Added line #L74 was not covered by tests
[warning] 77-79: components/factory/components/MyDenoms.tsx#L77-L79
Added lines #L77 - L79 were not covered by tests
[warning] 83-85: components/factory/components/MyDenoms.tsx#L83-L85
Added lines #L83 - L85 were not covered by tests
[warning] 100-100: components/factory/components/MyDenoms.tsx#L100
Added line #L100 was not covered by tests
[warning] 107-107: components/factory/components/MyDenoms.tsx#L107
Added line #L107 was not covered by tests
[warning] 121-126: components/factory/components/MyDenoms.tsx#L121-L126
Added lines #L121 - L126 were not covered by tests
[warning] 399-402: components/factory/components/MyDenoms.tsx#L399-L402
Added lines #L399 - L402 were not covered by tests
[warning] 406-407: components/factory/components/MyDenoms.tsx#L406-L407
Added lines #L406 - L407 were not covered by tests
🔇 Additional comments (6)
components/factory/components/MyDenoms.tsx (6)
100-100: Reassess Closing Transfer Modal in handleCloseModal
In handleCloseModal, you're setting setOpenTransferDenomModal(false);. Since this function is intended to close modals, this line is appropriate. Just ensure consistency with other modal close handlers.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 100-100: components/factory/components/MyDenoms.tsx#L100
Added line #L100 was not covered by tests
267-267: Pass Event Handler Correctly to TokenRow Component
The onTransfer prop is being passed correctly to the TokenRow component. Ensure that the handleTransferModal function properly handles the event.
Line range hint 424-438: Ensure Props and Handlers in TokenRow Are Consistent
The TokenRow component's props have been extended to include onTransfer, and the types align correctly. This addition is implemented properly.
396-414: Review TransferModal Integration
The TransferModal component is integrated correctly with the necessary props. Ensure that all callback functions and state variables (modalId, openTransferDenomModal, setOpenTransferDenomModal, onSuccess, etc.) are functioning as intended.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 399-402: components/factory/components/MyDenoms.tsx#L399-L402
Added lines #L399 - L402 were not covered by tests
[warning] 406-407: components/factory/components/MyDenoms.tsx#L406-L407
Added lines #L406 - L407 were not covered by tests
74-85:
Ensure Transfer Modal Opens When Action Is 'transfer'
In the useEffect hook, when the action is 'transfer', the TransferModal does not open because setOpenTransferDenomModal(true) is not called. This prevents users from accessing the transfer functionality via URL parameters.
Apply this diff to open the TransferModal when the action is 'transfer':
if (
action === 'mint' ||
action === 'burn' ||
action === 'multimint' ||
action === 'multiburn' ||
action === 'update' ||
+ action === 'transfer' ||
action === 'info'
) {
setModalType(
action as 'mint' | 'burn' | 'multimint' | 'multiburn' | 'update' | 'info' | 'transfer'
);
if (action === 'update') {
setOpenUpdateDenomMetadataModal(true);
}
+ if (action === 'transfer') {
+ setOpenTransferDenomModal(true);
+ }
}Likely invalid or redundant comment.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 74-74: components/factory/components/MyDenoms.tsx#L74
Added line #L74 was not covered by tests
[warning] 77-79: components/factory/components/MyDenoms.tsx#L77-L79
Added lines #L77 - L79 were not covered by tests
[warning] 83-85: components/factory/components/MyDenoms.tsx#L83-L85
Added lines #L83 - L85 were not covered by tests
490-497: Verify the Condition for Disabling the Transfer Button
The condition denom.base.includes('umfx') is used to disable the Transfer button. Verify that this condition correctly reflects when the transfer action should be disabled.
Run the following script to check all instances where denom.base includes 'umfx' and confirm that these are the tokens for which transfer should be disabled:
✅ Verification successful
Let me gather more context about the token functionality by checking the related code.
Based on the search results, I can now provide a definitive response about the condition for disabling the transfer button.
The umfx Token Disable Condition is Correct
The condition denom.base.includes('umfx') correctly identifies the native Manifest Chain token that should have transfer disabled. This is verified by:
- It's consistently used across multiple components to disable actions for the native token
- The token is defined as the native/core token in constants with base denom 'umfx'
- The same check is used to disable burn, mint and other token operations
- The token appears in configurations as the fee token and native denomination
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: List all denom bases that include 'umfx'
rg "denom\.base.*includes\('umfx'\)" -A 5
# Verify denom bases in the codebase
rg "base:.*'umfx'"
Length of output: 3690
Script:
#!/bin/bash
# Check for token-related functionality and permissions
ast-grep --pattern 'function $FUNC($$$) {
$$$
denom.base$$$
$$$
}'
# Check for any comments or documentation about umfx tokens
rg -i "umfx" -B 2 -A 2
# Check for any token type definitions or interfaces
ast-grep --pattern 'interface $_ {
base: $_
$$$
}'
Length of output: 76554
| onSuccess={() => { | ||
| refetchDenoms(); | ||
| handleUpdateModalClose(); | ||
| }} |
There was a problem hiding this comment.
Use handleCloseModal Instead of handleUpdateModalClose
In the onSuccess callback of the TransferModal, you're calling handleUpdateModalClose(), which is specific to the update modal. To properly close the transfer modal and reset relevant states, you should use handleCloseModal().
Apply this diff to correct the function call:
onSuccess={() => {
refetchDenoms();
- handleUpdateModalClose();
+ handleCloseModal();
}}Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 406-407: components/factory/components/MyDenoms.tsx#L406-L407
Added lines #L406 - L407 were not covered by tests
| router.push('/factory', undefined, { shallow: true }); | ||
| }; | ||
|
|
||
| const handleUpdateModalClose = () => { | ||
| setSelectedDenom(null); | ||
| setOpenUpdateDenomMetadataModal(false); | ||
| setOpenTransferDenomModal(false); |
There was a problem hiding this comment.
Avoid Closing Transfer Modal in handleUpdateModalClose
The handleUpdateModalClose function is specific to the update modal. Setting setOpenTransferDenomModal(false); here could lead to unintended side effects, such as closing the transfer modal when only the update modal should be affected.
Apply this diff to remove the unintended closure:
const handleUpdateModalClose = () => {
setSelectedDenom(null);
setOpenUpdateDenomMetadataModal(false);
- setOpenTransferDenomModal(false);
setModalType(null);
router.push('/factory', undefined, { shallow: true });
};Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 107-107: components/factory/components/MyDenoms.tsx#L107
Added line #L107 was not covered by tests
This PR allows one to transfer ownership of a token created via the Token Factory module to a new owner. The Factory page now only displays tokens owned by the current user.
manifestjswas bumped to0.0.1-rc.1to be able to query tokens by owner (admin).Fixes #135
Relates #137
Summary by CodeRabbit
Release Notes
New Features
TransferModalfor token transfers, allowing users to initiate ownership changes directly.Improvements
MyDenomscomponent to manage transfer modal visibility and state.useQuerieshook to include new functionality for fetching authority metadata.Dependency Updates