fix unstakeAll issue #888
fix unstakeAll issue #888
Conversation
WalkthroughThe changes primarily involve refactoring the Changes
Poem
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (2)
- packages/extension-polkagate/src/popup/staking/solo/unstake/Review.tsx (7 hunks)
- packages/extension-polkagate/src/popup/staking/solo/unstake/index.tsx (5 hunks)
Additional comments: 10
packages/extension-polkagate/src/popup/staking/solo/unstake/Review.tsx (6)
- 30-38: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [30-45]
The new prop
unstakeAllAmounthas been added. Ensure that all instances of this component have been updated to pass this prop.
47-54: The
api,chain, andformattedvalues are now being retrieved through custom hooks instead of being passed as props. This is a good practice as it reduces the number of props that need to be passed and makes the component more self-contained.86-92: The
unstakefunction has been updated to use theapi,chain, andformattedvalues retrieved from the custom hooks. This is a good practice as it makes the function more self-contained and less dependent on external values.105-111: The
unstakefunction now checks ifunstakeAllAmountis true andhasNominatoris true before pushingchilled()totxs. This is a logical change that should be correct ifunstakeAllAmountandhasNominatorare being correctly set elsewhere in the code.121-127: The
txInfoobject now includes theunstakeAllAmountvalue. This is a logical change that should be correct ifunstakeAllAmountis being correctly set elsewhere in the code.138-141: The
unstakefunction now setsisPasswordErrorto true if an error is caught. This is a good practice as it allows the UI to respond to the error.packages/extension-polkagate/src/popup/staking/solo/unstake/index.tsx (4)
17-21: The import of
Assethas been moved from line 3 to line 4. Ensure that this change does not affect other parts of the code that depend onAsset.52-57: The type of
stakingAccount.stakingLedger.activehas been changed toBNon line 52. Ensure that this change is compatible with the rest of the codebase. The calculation oftotalAfterUnstakeon line 54 has been updated to handledecimalbeing undefined. This is a good practice as it prevents potential runtime errors.104-110:
unstakeAllAmounthas been added as a dependency in theuseEffecton line 104. This is a good practice as it ensures that the effect will be run wheneverunstakeAllAmountchanges.114-120:
unstakeAllAmounthas been added as a dependency in the seconduseEffecton line 124. This is a good practice as it ensures that the effect will be run wheneverunstakeAllAmountchanges.
| <Review | ||
| address={address} | ||
| amount={amount} | ||
| api={api} | ||
| chain={chain} | ||
| chilled={chilled} | ||
| estimatedFee={estimatedFee} | ||
| formatted={formatted} | ||
| hasNominator={!!stakingAccount?.nominators?.length} | ||
| maxUnlockingChunks={maxUnlockingChunks} | ||
| redeem={redeem} | ||
| redeemDate={redeemDate} | ||
| setShow={setShowReview} | ||
| show={showReview} | ||
| staked={staked} | ||
| total={totalAfterUnstake} | ||
| unbonded={unbonded} | ||
| unlockingLen={unlockingLen ?? 0} | ||
| unstakeAllAmount={unstakeAllAmount} | ||
| /> |
There was a problem hiding this comment.
api and chain have been removed from the props passed to Review component on line 222. unstakeAllAmount has been added as a prop to the Review component on line 222. Ensure that the Review component has been updated to handle these changes.
- api={api}
- chain={chain}
+ unstakeAllAmount={unstakeAllAmount}Commitable suggestion
[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.
| <Review | |
| address={address} | |
| amount={amount} | |
| api={api} | |
| chain={chain} | |
| chilled={chilled} | |
| estimatedFee={estimatedFee} | |
| formatted={formatted} | |
| hasNominator={!!stakingAccount?.nominators?.length} | |
| maxUnlockingChunks={maxUnlockingChunks} | |
| redeem={redeem} | |
| redeemDate={redeemDate} | |
| setShow={setShowReview} | |
| show={showReview} | |
| staked={staked} | |
| total={totalAfterUnstake} | |
| unbonded={unbonded} | |
| unlockingLen={unlockingLen ?? 0} | |
| unstakeAllAmount={unstakeAllAmount} | |
| /> | |
| <Review | |
| address={address} | |
| amount={amount} | |
| chilled={chilled} | |
| estimatedFee={estimatedFee} | |
| hasNominator={!!stakingAccount?.nominators?.length} | |
| maxUnlockingChunks={maxUnlockingChunks} | |
| redeem={redeem} | |
| redeemDate={redeemDate} | |
| setShow={setShowReview} | |
| show={showReview} | |
| total={totalAfterUnstake} | |
| unbonded={unbonded} | |
| unlockingLen={unlockingLen ?? 0} | |
| unstakeAllAmount={unstakeAllAmount} | |
| /> |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (2)
- packages/extension-polkagate/src/popup/staking/solo/unstake/Review.tsx (7 hunks)
- packages/extension-polkagate/src/popup/staking/solo/unstake/index.tsx (8 hunks)
Additional comments: 12
packages/extension-polkagate/src/popup/staking/solo/unstake/index.tsx (6)
48-52: The state variable
unstakeAllAmounthas been replaced withisUnstakeAll. Ensure that the logic has been updated to handle this new state variable throughout the codebase.81-89: The logic has been updated to handle the
isUnstakeAllstate variable. Ensure that this logic is correct and that it behaves as expected.107-109: The
isUnstakeAllstate variable is used in thegetFeefunction. Ensure that this logic is correct and that it behaves as expected.133-134: The
isUnstakeAllstate variable is set tofalsewhen the amount changes. Ensure that this is the intended behavior.151-152: The
isUnstakeAllstate variable is set totruewhen theonAllAmountfunction is called. Ensure that this is the intended behavior.219-234: The
isUnstakeAllprop has been added to theReviewcomponent. Ensure that this prop is being used correctly in theReviewcomponent.packages/extension-polkagate/src/popup/staking/solo/unstake/Review.tsx (6)
- 30-38: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [30-45]
Ensure that all calls to this component throughout the codebase have been updated to match the new props.
47-54: The use of hooks to replace props is a good practice for reducing prop drilling and improving code readability.
86-92: Ensure that all the required variables are defined and not null before proceeding with the unstake operation. This is a good practice for error prevention.
105-111: The conditional logic for pushing transactions into the
txsarray is clear and concise.121-127: The creation of the
infoobject is straightforward and well-structured.143-144: The use of
useCallbackfor the_onBackClickfunction is a good practice for performance optimization.
| console.log('error:', e); | ||
| setIsPasswordError(true); | ||
| } |
There was a problem hiding this comment.
Consider adding more detailed error handling and logging. This could include logging the error message to a monitoring service or displaying a user-friendly error message.
- console.log('error:', e);
+ console.error('Unstaking error:', e.message);Commitable suggestion
[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.
| console.log('error:', e); | |
| setIsPasswordError(true); | |
| } | |
| console.error('Unstaking error:', e.message); | |
| setIsPasswordError(true); | |
| } |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- packages/extension-polkagate/src/popup/staking/solo/unstake/Review.tsx (7 hunks)
Additional comments: 5
packages/extension-polkagate/src/popup/staking/solo/unstake/Review.tsx (5)
- 30-38: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [30-45]
The
Propsinterface has been updated to includeisUnstakeAll. Ensure that all instances where theReviewcomponent is used have been updated to include this new prop.
86-92: The
unstakefunction now checks ifapi,chain,formatted,unbonded,redeem,chilled, orhasNominatorare undefined before proceeding. This is a good practice as it prevents potential runtime errors.100-111: The logic for adding transactions to the
txsarray has been updated. Now, ifunlockingLenis greater than or equal tomaxUnlockingChunks, aredeemtransaction is added. IfisUnstakeAllis true andhasNominatoris also true, achilledtransaction is added. This change seems to be in line with the pull request's goal of replacingunstakeAllAmountwithisUnstakeAll.121-127: The
infoobject now includes adateproperty, which is set to the current date and time. This is a good addition as it provides more information about when the unstaking action was performed.135-140: The
unstakefunction now catches any errors that occur during its execution and setsisPasswordErrorto true. This is a good practice as it improves error handling.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- packages/extension-polkagate/src/popup/staking/solo/unstake/index.tsx (8 hunks)
Additional comments: 6
packages/extension-polkagate/src/popup/staking/solo/unstake/index.tsx (6)
48-51: The state variable
unstakeAllAmounthas been replaced withisUnstakeAll. Ensure that this change is reflected throughout the codebase.81-89: The logic related to
unstakeAllAmounthas been updated to useisUnstakeAll. Ensure that this change does not introduce any logical errors.107-109: The
isUnstakeAllstate variable is now being used in thegetFeefunction. Ensure that this change does not introduce any logical errors.133-134: The
onChangeAmountfunction has been updated to useisUnstakeAllinstead ofunstakeAllAmount. Ensure that this change does not introduce any logical errors.151-152: The
onAllAmountfunction has been updated to useisUnstakeAllinstead ofunstakeAllAmount. Ensure that this change does not introduce any logical errors.219-233: The
isUnstakeAllprop has been added to theReviewcomponent. Ensure that this prop is being used correctly in theReviewcomponent.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- packages/extension-polkagate/src/popup/staking/solo/unstake/Review.tsx (5 hunks)
- packages/extension-polkagate/src/popup/staking/solo/unstake/index.tsx (8 hunks)
Additional comments: 13
packages/extension-polkagate/src/popup/staking/solo/unstake/Review.tsx (6)
- 8-55: > Note: This review was outside of the patch, but no patch was found that overlapped with it. Original lines [48-46]
The new prop
isUnstakeAllhas been added to theReviewcomponent's props. Ensure that all instances of this component have been updated to pass this prop.
50-53: The
apiandchainprops have been replaced with custom hooksuseApianduseChain. This is a good practice as it reduces prop drilling and makes the component more self-contained.87-92: The
unstakefunction has been updated to use the newisUnstakeAllprop and the custom hooks forapiandchain. This is a good practice as it reduces prop drilling and makes the function more self-contained.101-112: The logic for unstaking has been updated to handle the new
isUnstakeAllprop. Ensure that this logic is correct and that it behaves as expected in all scenarios.122-128: The
fromfield in the error handling section has been updated to useString(formatted). This is a good practice as it ensures that thefromfield is always a string.136-142: The
unstakefunction has been updated to handle the newisUnstakeAllprop. Ensure that this logic is correct and that it behaves as expected in all scenarios.packages/extension-polkagate/src/popup/staking/solo/unstake/index.tsx (7)
48-52: The introduction of the
isUnstakeAllstate variable seems to be a part of the solution to the unstaking issue. Ensure that it is being used correctly throughout the component.53-57: The calculation of
totalAfterUnstakehas been updated to handledecimalbeing undefined. This is a good practice as it prevents potential runtime errors.78-89: The condition for setting the alert has been updated to include
isUnstakeAll. This seems to be a part of the solution to the unstaking issue. Ensure that the condition is correct and that it doesn't introduce any new issues.104-110: The condition for pushing
chilled()totxshas been updated to useisUnstakeAll. This seems to be a part of the solution to the unstaking issue. Ensure that the condition is correct and that it doesn't introduce any new issues.132-144: The
onChangeAmountfunction has been updated to setisUnstakeAlltofalse. This seems to be a part of the solution to the unstaking issue. Ensure that this change doesn't introduce any new issues.150-155: The
onAllAmountfunction has been updated to setisUnstakeAlltotrue. This seems to be a part of the solution to the unstaking issue. Ensure that this change doesn't introduce any new issues.221-236: The
Reviewcomponent no longer receivesapiandchainas props but now receivesstakedandisUnstakeAll. Ensure that all calls to this component throughout the codebase have been updated to match the new props.
because stakeAmount is rounded while showing to user hence to find if chill is needed we change the condition to use the boolean value of isUnstakeAll boolean value
Summary by CodeRabbit
ReviewandIndexcomponents in the staking/unstake section by utilizing custom hooks for data retrieval, improving code readability and maintainability.isUnstakeAllto better handle unstaking scenarios.unstakefunction.totalAfterUnstaketo handledecimalbeing undefined.