feat: add accept trade flow #2268
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
meelrossi
force-pushed
the
feat/accept-trade
branch
from
15a4552 to
42d108c
Compare
meelrossi
force-pushed
the
feat/accept-trade
branch
from
5bf3bf0 to
9a2d82a
Compare
meelrossi
force-pushed
the
feat/accept-trade
branch
from
9a2d82a to
36bf0ec
Compare
meelrossi
force-pushed
the
feat/accept-trade
branch
from
36bf0ec to
9e409ec
Compare
webapp/src/components/Bid/Bid.tsx
Outdated
| <Button primary onClick={() => history.push(locations.bid(bid.contractAddress, bid.tokenId))}> | ||
| {t('global.update')} | ||
| </Button> | ||
| {'bidAddress' in bid && ( |
There was a problem hiding this comment.
should we show a disabled state/message when the bidAddress is not present in the bid?
There was a problem hiding this comment.
if the bidaddress is not in the bid then it means that is one of the "new" bids created by a trade. This new bids can't be updated. The process requires the cancellation of the previous trade and hte addition of the new one. That is why the button should not even appear.
| @@ -108,7 +108,7 @@ const BidsTable = (props: Props) => { | |||
| handleSortByChange={(value: string) => setSortBy(value as BidSortBy)} | |||
| sortBy={sortBy} | |||
| /> | |||
| {showConfirmationModal.bid && showConfirmationModal.display ? ( | |||
| {showConfirmationModal.bid && showConfirmationModal.display && 'tokenId' in showConfirmationModal.bid ? ( | |||
There was a problem hiding this comment.
What if we refactor this check as isLegacyBid(bid), it might be faster to understand what's being asserted, right? If this function also uses the is keyword, it can help you correctly type each type of bid.
| @@ -99,7 +99,7 @@ export const cancelBidSuccess = (bid: Bid, txHash: string) => | |||
| action(CANCEL_BID_SUCCESS, { | |||
| bid, | |||
| ...buildTransactionPayload(bid.chainId, txHash, { | |||
| tokenId: bid.tokenId, | |||
| ...('tokenId' in bid ? { tokenId: bid.tokenId } : { itemId: bid.itemId }), | |||
There was a problem hiding this comment.
This change will affect the Transaction component in the ActivityPage, we might wat to add the support for these types of transactions as well.
| txHash = yield call( | ||
| sendTransaction as (contract: ContractData, contractMethodName: string, ...contractArguments: any[]) => Promise<string>, | ||
| offchainMarketplaceContract, | ||
| 'function accept(Trade[] calldata _trades) external;', | ||
| [tradeToAccept] |
There was a problem hiding this comment.
As a future refactor, we might want to consider moving this section of code to the service, with a dedicated acceptOfChain method. It will make it easier to test as well.
There was a problem hiding this comment.
Added one task to the backlog to handle this
| @@ -111,7 +98,44 @@ export async function getTradeSignature(trade: Omit<TradeCreation, 'signature'>) | |||
| beneficiary: asset.beneficiary | |||
| })) | |||
| } | |||
| } | |||
|
|
|||
| export function getTradeToAccept(trade: Trade) { | |||
There was a problem hiding this comment.
It would be importante to test these function to be sure that the trade is being built correctly, would you mind adding tests for them?
webapp/src/modules/bid/sagas.ts
Outdated
| sellerBids = ((yield call([marketplaceAPI, 'fetchBids'])) as Awaited<ReturnType<typeof marketplaceAPI.fetchBids>>).results // TODO: add seller filter | ||
| bidderBids = ( | ||
| (yield call([marketplaceAPI, 'fetchBids'], { bidder: address })) as Awaited<ReturnType<typeof marketplaceAPI.fetchBids>> | ||
| ).results |
There was a problem hiding this comment.
What if we make this request using yield all as the one below is doing? That should speed up the loading of the bids.
webapp/src/modules/bid/sagas.spec.ts
Outdated
| marketplaceAPIMock = { fetchTrade: jest.fn().mockResolvedValue(trade) } as unknown as jest.Mocked<MarketplaceAPI> | ||
| }) | ||
|
|
||
| it.only('should dispatch an action signaling the success of the action handling', () => { |
There was a problem hiding this comment.
A wild only has appeared
Add flow to accept bids from offchain marketplace