This repository was archived by the owner on Jun 24, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 54
Small refactor in #1639 #1857
Merged
alfetopito
merged 2 commits into
1334/activity-orders-from-api-rebased
from
1334/small-refactor-1
Nov 17, 2021
Merged
Small refactor in #1639 #1857
alfetopito
merged 2 commits into
1334/activity-orders-from-api-rebased
from
1334/small-refactor-1
Nov 17, 2021
Conversation
This file contains 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
This was referenced Nov 16, 2021
alfetopito
approved these changes
Nov 17, 2021
|
alfetopito
added a commit
that referenced
this pull request
Nov 29, 2021
* Added /account/<address>/orders api endpoint * Added helper function to get explorer address link * Requiring needed properties instead of concrete types on utils fns * Added addOrUpdateOrdersBatch to orders/actions * Added useAddOrUpdateOrdersBatch to orders/hooks * Added addOrUpdateOrdersBatch to orders/reducer * Added APIOrdersUpdater that checks for orders from API on connection * Adjusted displayed orders: - At most 10 regular orders and as much pending as there are - Show only connected account orders - Sort orders by creation date descending * Show only txs for connected account * Refactoring * Don't actually need a helper fn... * Added comment to make sort more explicit * Using 'Link to Explorer' instead of 'Clear activities' * Refactoring * Refactor: cache tokens loaded const * Refactor: removed duplicated css property * Refactor: removed duplicated CSS property * Refactor: extracted function transformApiOrderToStoreOrder * Refactor: renamed _orders to apiOrders * Refactor: removed duplicated CSS property * Adjusting `View all orders` link size * Added `View all orders` to the bottom of account activity * Refactored computeOrderSummary to deal with pending orders * Computing order summary for orders loaded from API * Setting isCancelling state if order is pending and cancelled on API * Actually syncing `isCancelling` flag for existing orders * Refactor: created const for magic number MAXIMUM_ORDERS_TO_DISPLAY * Refactor: simplified addOrUpdateOrdersBatch reducer * Refactor: renamed API -> Api on file name * Refactor: replaced magic number with AMOUNT_OF_ORDERS_TO_FETCH * Refactor: converted classifyLocalStatus fn to a map * Refactor: removed `Batch` from addOrUpdateOrders * Refactor: moved error exit condition up * 1334/load new tokens not yet loaded (#1504) * Added non-hook methods for dealing with contracts * New hook useTokensLazy * Exporting stuff from original codebase * Added getMultipleCallsResults under custom/state/multicall/utils Non-hook way of fetching multiple onchain queries through multicall * Refactor ApiOrdersUpdater to load tokens not yet in the UI * Typo fix * 1334/do not refetch when token added (#1536) * Removing annoying log * No longer re-fetching orders when a token is added to the list Co-authored-by: Leandro Boscariol <[email protected]> * Refactored useTokensLazy: - Added contracts cache - Improved comments * Improved doc string on multicall utils Co-authored-by: Leandro Boscariol <[email protected]> * Fix new Erc20 import path * Adjust to new multicall interface and fetchChunk changes * Adapt to renamed of ApiOrderStatus to OrderTransitionStatus * Adjusting (again) styles for `view all activity` * Handling presignaturePending state from backend * The order is concrete at this point, no need for `?.` * 1334/only update connected wallet (#1684) * Only update cancelled orders for connected wallet * Only check for unfillable orders for connected wallet * Only check pending orders for connected wallet * Always create a new state object when adding an order (#1686) Co-authored-by: Leandro Boscariol <[email protected]> * Refactor: comparing account with order.owner lowercased for safety Co-authored-by: Leandro Boscariol <[email protected]> * Added new hook useTokenLazyNoMulticall to fetch token data one by one * Using new hook to fetch tokens instead of multicall * Removed multicall/utils * Removed useTokensLazy hook * Renamed file useTokensLazy to useTokenLazy * Renamed useTokenLazyNoMulticall to useTokenLazy * Removed unused import * Updated debug log messages * Small refactor in #1639 (#1857) * Only init bytes32Contract if we really need to * Fixing build error: only proceed if library is set Co-authored-by: Leandro Boscariol <[email protected]> * 1334/with swr (#1906) * Refactor: removed unnecessary variable * Added SWR as dev dependency * Added hook useApiOrders using useSWR hook * Using new hook for fetching api orders * Removed unused imports... Co-authored-by: Leandro Boscariol <[email protected]> Co-authored-by: Leandro Boscariol <[email protected]> Co-authored-by: Anxo Rodriguez <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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
This is a small refactor i did trying to reduce the size and complexity of the token fetching logic.
useTokenLazy
is way smaller now, and the work has been moved to different private functions.I continued with the refactor further in another PR (#1858), however this other PR is more ambitious because it also tries to address the issue that #1639 doesn't use the
cancel
that is returned by theretry
function. So this part is not included in this PR.For both PR, @alfetopito u don't need to use my code. I just wanted to give it a try to refactor it, and understand it a bit better. Feel free to merge, take bits and pieces, use inspiration, or ignore all together these two PRs.