diff --git a/src/containers/CheckoutPage/CheckoutPage.duck.js b/src/containers/CheckoutPage/CheckoutPage.duck.js index 71d57d95..64871dca 100644 --- a/src/containers/CheckoutPage/CheckoutPage.duck.js +++ b/src/containers/CheckoutPage/CheckoutPage.duck.js @@ -1,5 +1,5 @@ import { pick } from 'lodash'; -import { updatedEntities, denormalisedEntities } from '../../util/data'; +import { denormalisedResponseEntities } from '../../util/data'; import { storableError } from '../../util/errors'; import { TRANSITION_REQUEST } from '../../util/types'; import * as log from '../../util/log'; @@ -176,10 +176,11 @@ export const speculateTransaction = (listingId, bookingStart, bookingEnd) => ( return sdk.transactions .initiateSpeculative(bodyParams, queryParams) .then(response => { - const transactionId = response.data.data.id; - const entities = updatedEntities({}, response.data); - const denormalised = denormalisedEntities(entities, 'transaction', [transactionId]); - const tx = denormalised[0]; + const entities = denormalisedResponseEntities(response); + if (entities.length !== 1) { + throw new Error('Expected a resource in the sdk.transactions.initiateSpeculative response'); + } + const tx = entities[0]; dispatch(speculateTransactionSuccess(tx)); }) .catch(e => { diff --git a/src/containers/ListingPage/ListingPage.duck.js b/src/containers/ListingPage/ListingPage.duck.js index 6c6b9d88..1e33acf7 100644 --- a/src/containers/ListingPage/ListingPage.duck.js +++ b/src/containers/ListingPage/ListingPage.duck.js @@ -2,7 +2,7 @@ import { pick } from 'lodash'; import { types as sdkTypes } from '../../util/sdkLoader'; import { storableError } from '../../util/errors'; import { addMarketplaceEntities } from '../../ducks/marketplaceData.duck'; -import { updatedEntities, denormalisedEntities } from '../../util/data'; +import { denormalisedResponseEntities } from '../../util/data'; import { TRANSITION_ENQUIRE } from '../../util/types'; import { fetchCurrentUser } from '../../ducks/user.duck'; @@ -117,10 +117,8 @@ export const fetchReviews = listingId => (dispatch, getState, sdk) => { return sdk.reviews .query({ listing_id: listingId, state: 'public', include: ['author', 'author.profileImage'] }) .then(response => { - const entities = updatedEntities({}, response.data); - const reviewIds = response.data.data.map(d => d.id); - const denormalized = denormalisedEntities(entities, 'review', reviewIds); - dispatch(fetchReviewsSuccess(denormalized)); + const reviews = denormalisedResponseEntities(response); + dispatch(fetchReviewsSuccess(reviews)); }) .catch(e => { dispatch(fetchReviewsError(storableError(e))); diff --git a/src/containers/ManageListingsPage/ManageListingsPage.duck.js b/src/containers/ManageListingsPage/ManageListingsPage.duck.js index 582a72aa..3e63e18a 100644 --- a/src/containers/ManageListingsPage/ManageListingsPage.duck.js +++ b/src/containers/ManageListingsPage/ManageListingsPage.duck.js @@ -34,10 +34,11 @@ const initialState = { const resultIds = data => data.data.map(l => l.id); -const merge = (state, apiResponse) => { +const merge = (state, sdkResponse) => { + const apiResponse = sdkResponse.data; return { ...state, - ownEntities: updatedEntities({ ...state.ownEntities }, apiResponse.data), + ownEntities: updatedEntities({ ...state.ownEntities }, apiResponse), }; }; @@ -143,35 +144,14 @@ export default manageListingsPageReducer; * @param {Object} state the full Redux store * @param {Array} listingIds listing IDs to select from the store */ -export const getListingsById = (state, listingIds) => { +export const getOwnListingsById = (state, listingIds) => { const { ownEntities } = state.ManageListingsPage; - try { - return denormalisedEntities(ownEntities, 'ownListing', listingIds); - } catch (e) { - return []; - } -}; - -/** - * Get the denormalised own entities from the given entity references. - * - * @param {Object} state the full Redux store - * - * @param {Array<{ id, type }} entityRefs References to entities that - * we want to query from the data. Currently we expect that all the - * entities have the same type. - * - * @return {Array} denormalised entities - */ -export const getOwnEntities = (state, entityRefs) => { - const { ownEntities } = state.ManageListingsPage; - const type = entityRefs.length > 0 ? entityRefs[0].type : null; - const ids = entityRefs.map(ref => ref.id); - try { - return denormalisedEntities(ownEntities, type, ids); - } catch (e) { - return []; - } + const resources = listingIds.map(id => ({ + id, + type: 'ownListing', + })); + const throwIfNotFound = false; + return denormalisedEntities(ownEntities, resources, throwIfNotFound); }; // ================ Action creators ================ // @@ -179,9 +159,9 @@ export const getOwnEntities = (state, entityRefs) => { // This works the same way as addMarketplaceEntities, // but we don't want to mix own listings with searched listings // (own listings data contains different info - e.g. exact location etc.) -export const addOwnEntities = apiResponse => ({ +export const addOwnEntities = sdkResponse => ({ type: ADD_OWN_ENTITIES, - payload: apiResponse, + payload: sdkResponse, }); export const openListingRequest = listingId => ({ diff --git a/src/containers/ManageListingsPage/ManageListingsPage.js b/src/containers/ManageListingsPage/ManageListingsPage.js index 42d59d13..85f4245e 100644 --- a/src/containers/ManageListingsPage/ManageListingsPage.js +++ b/src/containers/ManageListingsPage/ManageListingsPage.js @@ -22,7 +22,7 @@ import { TopbarContainer } from '../../containers'; import { closeListing, openListing, - getListingsById, + getOwnListingsById, queryOwnListings, } from './ManageListingsPage.duck'; import css from './ManageListingsPage.css'; @@ -201,7 +201,7 @@ const mapStateToProps = state => { closingListing, closingListingError, } = state.ManageListingsPage; - const listings = getListingsById(state, currentPageResultIds); + const listings = getOwnListingsById(state, currentPageResultIds); return { currentPageResultIds, listings, diff --git a/src/containers/ProfilePage/ProfilePage.duck.js b/src/containers/ProfilePage/ProfilePage.duck.js index 58ac3b6d..1d34d324 100644 --- a/src/containers/ProfilePage/ProfilePage.duck.js +++ b/src/containers/ProfilePage/ProfilePage.duck.js @@ -1,6 +1,6 @@ import { addMarketplaceEntities } from '../../ducks/marketplaceData.duck'; import { fetchCurrentUser } from '../../ducks/user.duck'; -import { updatedEntities, denormalisedEntities } from '../../util/data'; +import { denormalisedResponseEntities } from '../../util/data'; import { storableError } from '../../util/errors'; // ================ Action types ================ // @@ -139,10 +139,8 @@ export const queryUserReviews = userId => (dispatch, getState, sdk) => { sdk.reviews .query({ subject_id: userId, state: 'public', include: ['author', 'author_profile_image'] }) .then(response => { - const entities = updatedEntities({}, response.data); - const reviewIds = response.data.data.map(d => d.id); - const denormalized = denormalisedEntities(entities, 'review', reviewIds); - dispatch(queryReviewsSuccess(denormalized)); + const reviews = denormalisedResponseEntities(response); + dispatch(queryReviewsSuccess(reviews)); }) .catch(e => dispatch(queryReviewsError(e))); }; diff --git a/src/containers/ProfileSettingsPage/ProfileSettingsPage.duck.js b/src/containers/ProfileSettingsPage/ProfileSettingsPage.duck.js index 1b808cc3..65328756 100644 --- a/src/containers/ProfileSettingsPage/ProfileSettingsPage.duck.js +++ b/src/containers/ProfileSettingsPage/ProfileSettingsPage.duck.js @@ -1,4 +1,4 @@ -import { updatedEntities, denormalisedEntities } from '../../util/data'; +import { denormalisedResponseEntities } from '../../util/data'; import { storableError } from '../../util/errors'; import { currentUserShowSuccess } from '../../ducks/user.duck'; @@ -134,11 +134,11 @@ export const updateProfile = actionPayload => { .then(response => { dispatch(updateProfileSuccess(response)); - // Include profile image to denormalized user entity - const currentUserId = response.data.data.id; - const entities = updatedEntities({}, response.data); - const denormalised = denormalisedEntities(entities, 'currentUser', [currentUserId]); - const currentUser = denormalised[0]; + const entities = denormalisedResponseEntities(response); + if (entities.length !== 1) { + throw new Error('Expected a resource in the sdk.currentUser.updateProfile response'); + } + const currentUser = entities[0]; // Update current user in state.user.currentUser through user.duck.js dispatch(currentUserShowSuccess(currentUser)); diff --git a/src/containers/TransactionPage/TransactionPage.duck.js b/src/containers/TransactionPage/TransactionPage.duck.js index ea007131..fdab3ba9 100644 --- a/src/containers/TransactionPage/TransactionPage.duck.js +++ b/src/containers/TransactionPage/TransactionPage.duck.js @@ -10,7 +10,11 @@ import { TRANSITION_REVIEW_2_BY_PROVIDER, } from '../../util/types'; import * as log from '../../util/log'; -import { updatedEntities, denormalisedEntities } from '../../util/data'; +import { + updatedEntities, + denormalisedEntities, + denormalisedResponseEntities, +} from '../../util/data'; import { addMarketplaceEntities } from '../../ducks/marketplaceData.duck'; import { fetchCurrentUserNotifications } from '../../ducks/user.duck'; @@ -221,7 +225,8 @@ export const fetchTransaction = id => (dispatch, getState, sdk) => { txResponse = response; const listingId = listingRelationship(response).id; const entities = updatedEntities({}, response.data); - const denormalised = denormalisedEntities(entities, 'listing', [listingId]); + const listingRef = { id: listingId, type: 'listing' }; + const denormalised = denormalisedEntities(entities, [listingRef]); const listing = denormalised[0]; const canFetchListing = listing && listing.attributes && !listing.attributes.deleted; @@ -302,15 +307,13 @@ const fetchMessages = (txId, page) => (dispatch, getState, sdk) => { return sdk.messages .query({ transaction_id: txId, include: ['sender', 'sender.profileImage'], ...paging }) .then(response => { - const entities = updatedEntities({}, response.data); - const messageIds = response.data.data.map(d => d.id); - const denormalizedMessages = denormalisedEntities(entities, 'message', messageIds); + const messages = denormalisedResponseEntities(response); const { totalItems, totalPages, page: fetchedPage } = response.data.meta; const pagination = { totalItems, totalPages, page: fetchedPage }; const totalMessages = getState().TransactionPage.totalMessages; // Original fetchMessages call succeeded - dispatch(fetchMessagesSuccess(denormalizedMessages, pagination)); + dispatch(fetchMessagesSuccess(messages, pagination)); // Check if totalItems has changed between fetched pagination pages // if totalItems has changed, fetch first page again to include new incoming messages. diff --git a/src/ducks/marketplaceData.duck.js b/src/ducks/marketplaceData.duck.js index e15919e0..5ae2e039 100644 --- a/src/ducks/marketplaceData.duck.js +++ b/src/ducks/marketplaceData.duck.js @@ -11,10 +11,11 @@ const initialState = { entities: {}, }; -const merge = (state, apiResponse) => { +const merge = (state, sdkResponse) => { + const apiResponse = sdkResponse.data; return { ...state, - entities: updatedEntities({ ...state.entities }, apiResponse.data), + entities: updatedEntities({ ...state.entities }, apiResponse), }; }; @@ -39,11 +40,12 @@ export default function marketplaceDataReducer(state = initialState, action = {} */ export const getListingsById = (state, listingIds) => { const { entities } = state.marketplaceData; - try { - return denormalisedEntities(entities, 'listing', listingIds); - } catch (e) { - return []; - } + const resources = listingIds.map(id => ({ + id, + type: 'listing', + })); + const throwIfNotFound = false; + return denormalisedEntities(entities, resources, throwIfNotFound); }; /** @@ -59,18 +61,13 @@ export const getListingsById = (state, listingIds) => { */ export const getMarketplaceEntities = (state, entityRefs) => { const { entities } = state.marketplaceData; - const type = entityRefs.length > 0 ? entityRefs[0].type : null; - const ids = entityRefs.map(ref => ref.id); - try { - return denormalisedEntities(entities, type, ids); - } catch (e) { - return []; - } + const throwIfNotFound = false; + return denormalisedEntities(entities, entityRefs, throwIfNotFound); }; // ================ Action creators ================ // -export const addMarketplaceEntities = apiResponse => ({ +export const addMarketplaceEntities = sdkResponse => ({ type: ADD_MARKETPLACE_ENTITIES, - payload: apiResponse, + payload: sdkResponse, }); diff --git a/src/ducks/user.duck.js b/src/ducks/user.duck.js index 4e867608..45805c41 100644 --- a/src/ducks/user.duck.js +++ b/src/ducks/user.duck.js @@ -1,4 +1,4 @@ -import { updatedEntities, denormalisedEntities } from '../util/data'; +import { denormalisedResponseEntities } from '../util/data'; import { storableError } from '../util/errors'; import { TRANSITION_REQUEST, TRANSITION_REQUEST_AFTER_ENQUIRY } from '../util/types'; import * as log from '../util/log'; @@ -335,11 +335,12 @@ export const fetchCurrentUser = () => (dispatch, getState, sdk) => { return sdk.currentUser .show({ include: ['profileImage'] }) .then(response => { - // Include profile image to denormalized user entity - const currentUserId = response.data.data.id; - const entities = updatedEntities({}, response.data); - const denormalised = denormalisedEntities(entities, 'currentUser', [currentUserId]); - const currentUser = denormalised[0]; + const entities = denormalisedResponseEntities(response); + if (entities.length !== 1) { + throw new Error('Expected a resource in the sdk.currentUser.show response'); + } + const currentUser = entities[0]; + // set current user id to the logger log.setUserId(currentUser.id.uuid); dispatch(currentUserShowSuccess(currentUser)); diff --git a/src/util/data.js b/src/util/data.js index 70c5b13c..470aa8ec 100644 --- a/src/util/data.js +++ b/src/util/data.js @@ -39,7 +39,6 @@ export const updatedEntities = (oldEntities, apiResponse) => { const { data, included = [] } = apiResponse; const objects = (Array.isArray(data) ? data : [data]).concat(included); - /* eslint-disable no-param-reassign */ const newEntities = objects.reduce((entities, curr) => { const { id, type } = curr; entities[type] = entities[type] || {}; @@ -47,30 +46,36 @@ export const updatedEntities = (oldEntities, apiResponse) => { entities[type][id.uuid] = entity ? combinedResourceObjects(entity, curr) : curr; return entities; }, oldEntities); - /* eslint-enable no-param-reassign */ return newEntities; }; /** - * Denormalise the entities with the given IDs from the entities object + * Denormalise the entities with the resources from the entities object * * This function calculates the dernormalised tree structure from the * normalised entities object with all the relationships joined in. * * @param {Object} entities entities object in the SDK Redux store - * @param {String} type entity type of the given IDs - * @param {Array} ids IDs to pick from the entities + * @param {Array<{ id, type }} resources array of objects + * with id and type + * @param {Boolean} throwIfNotFound wheather to skip a resource that + * is not found (false), or to throw an Error (true) + * + * @return {Array} the given resource objects denormalised that were + * found in the entities */ -export const denormalisedEntities = (entities, type, ids) => { - if (!entities[type] && ids.length > 0) { - throw new Error(`No entities of type ${type}`); - } - return ids.map(id => { - const entity = entities[type][id.uuid]; - if (!entity) { - throw new Error(`Entity ${type} with id ${id.uuid} not found`); +export const denormalisedEntities = (entities, resources, throwIfNotFound = true) => { + const denormalised = resources.map(res => { + const { id, type } = res; + const entityFound = entities[type] && id && entities[type][id.uuid]; + if (!entityFound) { + if (throwIfNotFound) { + throw new Error(`Entity with type "${type}" and id "${id ? id.uuid : id}" not found`); + } + return null; } + const entity = entities[type][id.uuid]; const { relationships, ...entityData } = entity; if (relationships) { @@ -84,15 +89,13 @@ export const denormalisedEntities = (entities, type, ids) => { const hasMultipleRefs = Array.isArray(relRef.data); const multipleRefsEmpty = hasMultipleRefs && relRef.data.length === 0; if (!relRef.data || multipleRefsEmpty) { - // eslint-disable-next-line no-param-reassign ent[relName] = hasMultipleRefs ? [] : null; } else { const refs = hasMultipleRefs ? relRef.data : [relRef.data]; - const relIds = refs.map(ref => ref.id); - const relType = refs[0].type; - const rels = denormalisedEntities(entities, relType, relIds); - // eslint-disable-next-line no-param-reassign + // If a relationship is not found, an Error should be thrown + const rels = denormalisedEntities(entities, refs, true); + ent[relName] = hasMultipleRefs ? rels : rels[0]; } return ent; @@ -102,6 +105,28 @@ export const denormalisedEntities = (entities, type, ids) => { } return entityData; }); + return denormalised.filter(e => !!e); +}; + +/** + * Denormalise the data from the given SDK response + * + * @param {Object} sdkResponse response object from an SDK call + * + * @return {Array} entities in the response with relationships + * denormalised from the included data + */ +export const denormalisedResponseEntities = sdkResponse => { + const apiResponse = sdkResponse.data; + const data = apiResponse.data; + const resources = Array.isArray(data) ? data : [data]; + + if (!data || resources.length === 0) { + return []; + } + + const entities = updatedEntities({}, apiResponse); + return denormalisedEntities(entities, resources); }; /** diff --git a/src/util/data.test.js b/src/util/data.test.js index 5368fa35..013cb0bc 100644 --- a/src/util/data.test.js +++ b/src/util/data.test.js @@ -187,38 +187,43 @@ describe('data utils', () => { attributes: { url: `https://example.com/${id}.jpg`, width: 300, height: 200 }, }); - it('return no results with empty ids', () => { + it('returns no results with empty resources', () => { const entities = { listing: { listing1: createListing('listing1') } }; - expect(denormalisedEntities(entities, 'listing', [])).toEqual([]); + expect(denormalisedEntities(entities, [])).toEqual([]); }); - it('return no results with empty entities and empty ids', () => { + it('returns no results with empty entities and empty ids', () => { const entities = {}; - expect(denormalisedEntities(entities, 'listing', [])).toEqual([]); + expect(denormalisedEntities(entities, [])).toEqual([]); }); - it('throws when selecting a nonexistent type', () => { + it('handles selecting a nonexistent resource of different type', () => { const entities = { listing: { listing1: createListing('listing1') } }; - expect(() => denormalisedEntities(entities, 'user', [new UUID('user1')])).toThrow( - 'No entities of type user' + const user = createUser('user1'); + expect(() => denormalisedEntities(entities, [user])).toThrow( + 'Entity with type "user" and id "user1" not found' ); + // Empty results when error throwIfNotFound=false + expect(denormalisedEntities(entities, [user], false)).toEqual([]); }); - it('throws when selecting a nonexistent id', () => { + it('handles selecting a nonexistent resource of same type', () => { const entities = { listing: { listing1: createListing('listing1') } }; - const ids = [new UUID('listing2')]; - expect(() => denormalisedEntities(entities, 'listing', ids)).toThrow( - 'Entity listing with id listing2 not found' + const listing2 = createListing('listing2'); + expect(() => denormalisedEntities(entities, [listing2])).toThrow( + 'Entity with type "listing" and id "listing2" not found' ); + // Empty results when error throwIfNotFound=false + expect(denormalisedEntities(entities, [listing2], false)).toEqual([]); }); it('throws if a related entity is not found', () => { const listing1 = createListing('listing1'); listing1.relationships = { author: { data: createUser('user1') } }; const entities = { listing: { listing1 } }; - const ids = [listing1.id]; - expect(() => denormalisedEntities(entities, 'listing', ids)).toThrow( - 'No entities of type user' + const resources = [listing1]; + expect(() => denormalisedEntities(entities, resources)).toThrow( + 'Entity with type "user" and id "user1" not found' ); }); @@ -228,8 +233,39 @@ describe('data utils', () => { const listing3 = createListing('listing3'); const entities = { listing: { listing1, listing2, listing3 } }; - const ids = [listing1.id, listing3.id]; - expect(denormalisedEntities(entities, 'listing', ids)).toEqual([listing1, listing3]); + const resources = [listing1, listing3]; + expect(denormalisedEntities(entities, resources)).toEqual([listing1, listing3]); + }); + + it('returns entities of different types', () => { + const user1 = createUser('user1'); + const listing1 = createListing('listing1'); + const entities = { + listing: { listing1 }, + user: { user1 }, + }; + expect(denormalisedEntities(entities, [listing1, user1])).toEqual([listing1, user1]); + }); + + it('throws with circular relationships', () => { + // Currently the data handling code cannot handle circular + // relationships. These are possible in practice with certain + // include params in different API queries, but fixing this is + // not trivial. The denormalisation cannot skip entities that + // are already added since that would remove the fundamental + // feature of updating entities with new data. + + const user1 = createUser('user1'); + const listing1 = createListing('listing1'); + const listing1Relationships = { author: { data: user1 } }; + const listing1WithRelationships = { ...listing1, relationships: listing1Relationships }; + const user1Relationships = { topListing: { data: listing1 } }; + const user1WithRelationships = { ...user1, relationships: user1Relationships }; + const entities = { + listing: { listing1: listing1WithRelationships }, + user: { user1: user1WithRelationships }, + }; + expect(() => denormalisedEntities(entities, [listing1])).toThrow(); }); it('denormalises simple relationships', () => { @@ -242,10 +278,7 @@ describe('data utils', () => { listing: { listing1: listing1WithRelationships, listing2 }, user: { user1 }, }; - const ids = [listing1.id]; - expect(denormalisedEntities(entities, 'listing', ids)).toEqual([ - { ...listing1, author: user1 }, - ]); + expect(denormalisedEntities(entities, [listing1])).toEqual([{ ...listing1, author: user1 }]); }); it('denormalises multiple relationships', () => { @@ -265,9 +298,8 @@ describe('data utils', () => { user: { user1 }, image: { image1, image2 }, }; - const ids = [listing1.id, listing2.id]; - expect(denormalisedEntities(entities, 'listing', ids)).toEqual([ + expect(denormalisedEntities(entities, [listing1, listing2])).toEqual([ { ...listing1, author: user1, images: [image1, image2] }, listing2, ]); @@ -287,9 +319,8 @@ describe('data utils', () => { listing: { listing1: listing1WithRelationships, listing2, listing3 }, user: { user1 }, }; - const ids = [listing1.id, listing2.id]; - expect(denormalisedEntities(entities, 'listing', ids)).toEqual([ + expect(denormalisedEntities(entities, [listing1, listing2])).toEqual([ { ...listing1, author: null, images: [] }, listing2, ]);