Skip to content

Commit

Permalink
fix: do not fetch assets if they are in the store (#1734)
Browse files Browse the repository at this point in the history
* fix: do not fetch assets if they are in the store

* chore: remove useEffect unneeded dep

* test: update tests

* test: add tests and fix typo

* test: fix broken test
  • Loading branch information
juanmahidalgo authored May 26, 2023
1 parent f3b9dc9 commit fec0b0a
Show file tree
Hide file tree
Showing 16 changed files with 332 additions and 55 deletions.
11 changes: 9 additions & 2 deletions webapp/src/components/AssetBrowse/AssetBrowse.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React, { ReactNode, useEffect, useState } from 'react'
import { matchPath } from 'react-router-dom'
import { matchPath, useHistory, useLocation } from 'react-router-dom'
import classNames from 'classnames'
import { Container, Mobile, NotMobile, Page, Tabs } from 'decentraland-ui'
import { t } from 'decentraland-dapps/dist/modules/translation/utils'
Expand Down Expand Up @@ -53,8 +53,15 @@ const AssetBrowse = (props: Props) => {
isMapViewFiltersEnabled
} = props

const location = useLocation()
const history = useHistory()
// Prevent fetching more than once while browsing
const [hasFetched, setHasFetched] = useState(false)
const lastLocation = visitedLocations[visitedLocations.length - 2]
const [hasFetched, setHasFetched] = useState(
history.action === 'POP' &&
lastLocation?.pathname === location.pathname &&
lastLocation?.search === location.search
)
const isCurrentAccount = view === View.CURRENT_ACCOUNT
const isAccountOrCurrentAccount = view === View.ACCOUNT || isCurrentAccount
const [showOwnedLandOnMap, setShowOwnedLandOnMap] = useState(true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
isFetchingCollection,
getError as getCollectionError
} from '../../modules/collection/selectors'
import { fetchItemsRequest } from '../../modules/item/actions'
import { fetchCollectionItemsRequest } from '../../modules/item/actions'
import {
getItemsByContractAddress,
isFetchingItemsOfCollection
Expand Down Expand Up @@ -46,11 +46,9 @@ const mapDispatch = (
dispatch(fetchSingleCollectionRequest(contractAddress)),
onFetchCollectionItems: (collection: Collection) =>
dispatch(
fetchItemsRequest({
filters: {
first: collection.size,
contractAddresses: [collection.contractAddress]
}
fetchCollectionItemsRequest({
first: collection.size,
contractAddresses: [collection.contractAddress]
})
)
})
Expand Down
14 changes: 8 additions & 6 deletions webapp/src/modules/collection/sagas.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { select } from '@redux-saga/core/effects'
import { expectSaga } from 'redux-saga-test-plan'
import { call } from 'redux-saga-test-plan/matchers'
import { fetchItemsRequest } from '../item/actions'
import { fetchCollectionItemsRequest } from '../item/actions'
import { getItemsByContractAddress } from '../item/selectors'
import { collectionAPI } from '../vendor/decentraland'
import {
Expand Down Expand Up @@ -52,7 +52,7 @@ describe('when handling a fetch collections request', () => {
})
})
describe('when should fetch items argument is true', () => {
it('should put a fetch items request action for each collection', () => {
it('should put a fetch collection items request action for each collection', () => {
const filters = {}
const contractAddress1 = 'contract address 1'
const contractAddress2 = 'contract address 2'
Expand Down Expand Up @@ -94,14 +94,16 @@ describe('when handling a fetch collections request', () => {
.put(fetchCollectionsSuccess(collections as any, 100))
// Fetches items for contract address 2 because sizes are different
.put(
fetchItemsRequest({
filters: { contractAddresses: [contractAddress2], first: size }
fetchCollectionItemsRequest({
contractAddresses: [contractAddress2],
first: size
})
)
// Fetches items for contract address 3 because there are no items for that collection stored
.put(
fetchItemsRequest({
filters: { contractAddresses: [contractAddress3], first: size }
fetchCollectionItemsRequest({
contractAddresses: [contractAddress3],
first: size
})
)
.dispatch(fetchCollectionsRequest(filters, true))
Expand Down
18 changes: 7 additions & 11 deletions webapp/src/modules/collection/sagas.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { call, takeEvery, put, select } from '@redux-saga/core/effects'
import { t } from 'decentraland-dapps/dist/modules/translation/utils'
import { isErrorWithMessage } from '../../lib/error'
import { fetchItemsRequest } from '../item/actions'
import { fetchCollectionItemsRequest } from '../item/actions'
import { getItemsByContractAddress } from '../item/selectors'
import { collectionAPI } from '../vendor/decentraland'
import { CollectionResponse } from '../vendor/decentraland/collection/types'
Expand Down Expand Up @@ -47,11 +47,9 @@ export function* handleFetchCollectionsRequest(

if (!items || items.length !== collection.size) {
yield put(
fetchItemsRequest({
filters: {
first: collection.size,
contractAddresses: [collection.contractAddress]
}
fetchCollectionItemsRequest({
first: collection.size,
contractAddresses: [collection.contractAddress]
})
)
}
Expand Down Expand Up @@ -97,11 +95,9 @@ export function* handleFetchSingleCollectionRequest(

if (!items || items.length !== collection.size) {
yield put(
fetchItemsRequest({
filters: {
first: collection.size,
contractAddresses: [collection.contractAddress]
}
fetchCollectionItemsRequest({
first: collection.size,
contractAddresses: [collection.contractAddress]
})
)
}
Expand Down
41 changes: 40 additions & 1 deletion webapp/src/modules/item/actions.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,13 @@ import {
FETCH_ITEMS_FAILURE,
FETCH_ITEMS_REQUEST,
FETCH_ITEMS_SUCCESS,
FETCH_TRENDING_ITEMS_REQUEST
FETCH_TRENDING_ITEMS_REQUEST,
fetchCollectionItemsRequest,
FETCH_COLLECTION_ITEMS_REQUEST,
fetchCollectionItemsSuccess,
FETCH_COLLECTION_ITEMS_SUCCESS,
fetchCollectionItemsFailure,
FETCH_COLLECTION_ITEMS_FAILURE
} from './actions'

const itemBrowseOptions = {
Expand Down Expand Up @@ -200,3 +206,36 @@ describe('when creating the action to fetch the trending items request', () => {
})
})
})

describe('when creating the action to signal the start of the collection items request', () => {
let first: number
let contractAddresses: string[]
it('should return an object representing the action', () => {
expect(fetchCollectionItemsRequest({ first, contractAddresses })).toEqual({
type: FETCH_COLLECTION_ITEMS_REQUEST,
meta: undefined,
payload: { first, contractAddresses }
})
})
})

describe('when creating the action to signal a success in the collection items request', () => {
const items = [{} as Item]
it('should return an object representing the action', () => {
expect(fetchCollectionItemsSuccess(items)).toEqual({
type: FETCH_COLLECTION_ITEMS_SUCCESS,
meta: undefined,
payload: { items }
})
})
})

describe('when creating the action to signal a failure in the collection items request', () => {
it('should return an object representing the action', () => {
expect(fetchCollectionItemsFailure(anErrorMessage)).toEqual({
type: FETCH_COLLECTION_ITEMS_FAILURE,
meta: undefined,
payload: { error: anErrorMessage }
})
})
})
28 changes: 27 additions & 1 deletion webapp/src/modules/item/actions.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ChainId, Item } from '@dcl/schemas'
import { ChainId, Item, ItemFilters } from '@dcl/schemas'
import { NFTPurchase } from 'decentraland-dapps/dist/modules/gateway/types'
import {
buildTransactionPayload,
Expand Down Expand Up @@ -57,6 +57,32 @@ export type FetchTrendingItemsFailureAction = ReturnType<
typeof fetchTrendingItemsFailure
>

// Fetch Collection Items

export const FETCH_COLLECTION_ITEMS_REQUEST = '[Request] Fetch Collection Items'
export const FETCH_COLLECTION_ITEMS_SUCCESS = '[Success] Fetch Collection Items'
export const FETCH_COLLECTION_ITEMS_FAILURE = '[Failure] Fetch Collection Items'

export const fetchCollectionItemsRequest = (
options: Pick<ItemFilters, 'first' | 'contractAddresses'>
) => action(FETCH_COLLECTION_ITEMS_REQUEST, options)

export const fetchCollectionItemsSuccess = (items: Item[]) =>
action(FETCH_COLLECTION_ITEMS_SUCCESS, { items })

export const fetchCollectionItemsFailure = (error: string) =>
action(FETCH_COLLECTION_ITEMS_FAILURE, { error })

export type FetchCollectionItemsRequestAction = ReturnType<
typeof fetchCollectionItemsRequest
>
export type FetchCollectionItemsSuccessAction = ReturnType<
typeof fetchCollectionItemsSuccess
>
export type FetchCollectionItemsFailureAction = ReturnType<
typeof fetchCollectionItemsFailure
>

// Buy Item
export const BUY_ITEM_REQUEST = '[Request] Buy item'
export const BUY_ITEM_SUCCESS = '[Success] Buy item'
Expand Down
14 changes: 14 additions & 0 deletions webapp/src/modules/item/reducer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,12 @@ import {
buyItemWithCardFailure,
buyItemWithCardRequest,
buyItemWithCardSuccess,
FETCH_COLLECTION_ITEMS_SUCCESS,
FETCH_ITEM_SUCCESS,
FETCH_TRENDING_ITEMS_SUCCESS,
fetchCollectionItemsFailure,
fetchCollectionItemsRequest,
fetchCollectionItemsSuccess,
fetchItemFailure,
fetchItemRequest,
fetchItemsFailure,
Expand Down Expand Up @@ -79,6 +83,7 @@ const trendingItemsBatchSize = 20
const requestActions = [
fetchTrendingItemsRequest(trendingItemsBatchSize),
fetchItemsRequest(itemBrowseOptions),
fetchCollectionItemsRequest({ contractAddresses: [], first: 10 }),
fetchItemRequest(item.contractAddress, item.itemId),
buyItemRequest(item),
buyItemWithCardRequest(item),
Expand Down Expand Up @@ -115,6 +120,10 @@ const failureActions = [
request: fetchItemsRequest(itemBrowseOptions),
failure: fetchItemsFailure(anErrorMessage, itemBrowseOptions)
},
{
request: fetchCollectionItemsRequest({ contractAddresses: [], first: 10 }),
failure: fetchCollectionItemsFailure(anErrorMessage)
},
{
request: fetchItemRequest(item.contractAddress, item.itemId),
failure: fetchItemFailure(item.contractAddress, item.itemId, anErrorMessage)
Expand Down Expand Up @@ -192,6 +201,11 @@ describe('when reducing the successful action of fetching items', () => {
})

describe.each([
[
FETCH_COLLECTION_ITEMS_SUCCESS,
fetchCollectionItemsRequest({ contractAddresses: [], first: 10 }),
fetchCollectionItemsSuccess([item])
],
[
FETCH_ITEM_SUCCESS,
fetchItemRequest(item.contractAddress, item.itemId),
Expand Down
14 changes: 13 additions & 1 deletion webapp/src/modules/item/reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,13 @@ import {
BuyItemWithCardFailureAction,
BuyItemWithCardRequestAction,
BuyItemWithCardSuccessAction,
BUY_ITEM_WITH_CARD_FAILURE
BUY_ITEM_WITH_CARD_FAILURE,
FETCH_COLLECTION_ITEMS_SUCCESS,
FetchCollectionItemsRequestAction,
FetchCollectionItemsSuccessAction,
FetchCollectionItemsFailureAction,
FETCH_COLLECTION_ITEMS_REQUEST,
FETCH_COLLECTION_ITEMS_FAILURE
} from './actions'

export type ItemState = {
Expand Down Expand Up @@ -70,6 +76,9 @@ type ItemReducerAction =
| BuyItemWithCardRequestAction
| BuyItemWithCardSuccessAction
| BuyItemWithCardFailureAction
| FetchCollectionItemsRequestAction
| FetchCollectionItemsSuccessAction
| FetchCollectionItemsFailureAction

export function itemReducer(
state = INITIAL_STATE,
Expand All @@ -82,6 +91,7 @@ export function itemReducer(
case BUY_ITEM_WITH_CARD_SUCCESS:
case FETCH_ITEMS_REQUEST:
case FETCH_TRENDING_ITEMS_REQUEST:
case FETCH_COLLECTION_ITEMS_REQUEST:
case FETCH_ITEM_REQUEST: {
return {
...state,
Expand All @@ -90,6 +100,7 @@ export function itemReducer(
}
case FETCH_TRENDING_ITEMS_SUCCESS:
case FETCH_FAVORITED_ITEMS_SUCCESS:
case FETCH_COLLECTION_ITEMS_SUCCESS:
case FETCH_ITEMS_SUCCESS: {
const { items } = action.payload
return {
Expand Down Expand Up @@ -122,6 +133,7 @@ export function itemReducer(

case BUY_ITEM_FAILURE:
case BUY_ITEM_WITH_CARD_FAILURE:
case FETCH_COLLECTION_ITEMS_FAILURE:
case FETCH_ITEMS_FAILURE:
case FETCH_TRENDING_ITEMS_FAILURE:
case FETCH_ITEM_FAILURE: {
Expand Down
41 changes: 39 additions & 2 deletions webapp/src/modules/item/sagas.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@ import {
buyItemWithCardRequest,
buyItemWithCardFailure,
buyItemWithCardSuccess,
FETCH_ITEM_FAILURE
FETCH_ITEM_FAILURE,
fetchCollectionItemsRequest,
fetchCollectionItemsSuccess,
fetchCollectionItemsFailure
} from './actions'
import { itemSaga } from './sagas'
import { getData as getItems } from './selectors'
Expand Down Expand Up @@ -352,6 +355,40 @@ describe('when handling the set purchase action', () => {
})
})

describe('when handling the fetch collections items request action', () => {
describe('when the request is successful', () => {
const fetchResult = { data: [item] }

it('should dispatch a successful action with the fetched items', () => {
return expectSaga(itemSaga, getIdentity)
.provide([[matchers.call.fn(ItemAPI.prototype.get), fetchResult]])
.call.like({
fn: ItemAPI.prototype.get,
args: [{ first: 10, contractAddresses: [] }]
})
.put(fetchCollectionItemsSuccess(fetchResult.data))
.dispatch(
fetchCollectionItemsRequest({ contractAddresses: [], first: 10 })
)
.run({ silenceTimeout: true })
})
})

describe('when the request fails', () => {
it('should dispatch a failing action with the error and the options', () => {
return expectSaga(itemSaga, getIdentity)
.provide([
[matchers.call.fn(ItemAPI.prototype.get), Promise.reject(anError)]
])
.put(fetchCollectionItemsFailure(anError.message))
.dispatch(
fetchCollectionItemsRequest({ contractAddresses: [], first: 10 })
)
.run({ silenceTimeout: true })
})
})
})

describe('when handling the fetch items request action', () => {
describe('when the request is successful', () => {
let dateNowSpy: jest.SpyInstance
Expand Down Expand Up @@ -388,7 +425,7 @@ describe('when handling the fetch items request action', () => {
})

describe('when the request fails', () => {
it('should dispatching a failing action with the error and the options', () => {
it('should dispatch a failing action with the error and the options', () => {
return expectSaga(itemSaga, getIdentity)
.provide([
[matchers.call.fn(CatalogAPI.prototype.get), Promise.reject(anError)],
Expand Down
Loading

0 comments on commit fec0b0a

Please sign in to comment.