From 15f0325ec957dc729120eb94350e22ff38139d38 Mon Sep 17 00:00:00 2001 From: Chris Adams Date: Wed, 25 Nov 2020 10:33:31 +1100 Subject: [PATCH] Revert "Prevent pathological queries when doc is not specified (#784)" This reverts commit 046fd4775ad49eacdd8e9f59ed483a064531297c. This is an emergency fix to restore admin functionality. --- .eslintrc | 4 +- src/components/molecules/NavBar/NavBar.tsx | 28 +++---- src/hooks/roles.ts | 10 ++- src/hooks/useConnectCurrentVenueNG.ts | 4 +- src/hooks/useConnectRelatedVenues.ts | 26 +++---- src/hooks/useFirestoreConnect.ts | 86 ---------------------- src/hooks/useSparkleFirestoreConnect.ts | 59 +++++++++++++++ src/pages/Admin/Admin.tsx | 7 +- src/types/utility.ts | 5 -- 9 files changed, 99 insertions(+), 130 deletions(-) delete mode 100644 src/hooks/useFirestoreConnect.ts create mode 100644 src/hooks/useSparkleFirestoreConnect.ts diff --git a/.eslintrc b/.eslintrc index c13dbf2987..1181b3083e 100644 --- a/.eslintrc +++ b/.eslintrc @@ -22,8 +22,8 @@ "error", { "name": "react-redux", - "importNames": ["useSelector", "useDispatch", "useFirestoreConnect"], - "message": "Import typed version from hooks/{useSelector|useDispatch|useFirestoreConnect}" + "importNames": ["useSelector", "useDispatch"], + "message": "Import typed version from hooks/{useSelector|useDispatch}" } ], "react/prop-types": "off", diff --git a/src/components/molecules/NavBar/NavBar.tsx b/src/components/molecules/NavBar/NavBar.tsx index 9bc00ef83e..0cae48a3a7 100644 --- a/src/components/molecules/NavBar/NavBar.tsx +++ b/src/components/molecules/NavBar/NavBar.tsx @@ -1,4 +1,8 @@ import React, { useState, useMemo, useRef, useCallback } from "react"; +import { + ReduxFirestoreQuerySetting, + useFirestoreConnect, +} from "react-redux-firebase"; import { useHistory } from "react-router-dom"; import { OverlayTrigger, Popover } from "react-bootstrap"; import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; @@ -16,10 +20,6 @@ import { useRadio } from "hooks/useRadio"; import { useSelector } from "hooks/useSelector"; import { useUser } from "hooks/useUser"; import { useVenueId } from "hooks/useVenueId"; -import { - SparkleRFDocQuery, - useFirestoreConnect, -} from "hooks/useFirestoreConnect"; import AuthenticationModal from "components/organisms/AuthenticationModal"; import { GiftTicketModal } from "components/organisms/GiftTicketModal/GiftTicketModal"; @@ -89,18 +89,14 @@ const NavBar: React.FC = ({ redirectionUrl }) => { const radioStations = useSelector(radioStationsSelector); const parentVenue = useSelector(parentVenueSelector); - const venueParentQuery = useMemo( - () => - venue?.parentId - ? [ - { - collection: "venues", - doc: venue.parentId, - storeAs: "parentVenue", - }, - ] - : [], - [venue] + const venueParentId = venue?.parentId; + const venueParentQuery = useMemo( + () => ({ + collection: "venues", + doc: venueParentId, + storeAs: "parentVenue", + }), + [venueParentId] ); useFirestoreConnect(venueParentQuery); diff --git a/src/hooks/roles.ts b/src/hooks/roles.ts index 5e727a3903..809b8fa4ba 100644 --- a/src/hooks/roles.ts +++ b/src/hooks/roles.ts @@ -1,6 +1,10 @@ -import { isEmpty, isLoaded } from "react-redux-firebase"; +import { + isEmpty, + isLoaded, + ReduxFirestoreQuerySetting, + useFirestoreConnect, +} from "react-redux-firebase"; import { SparkleSelector } from "types/SparkleSelector"; -import { SparkleRFDocQuery, useFirestoreConnect } from "./useFirestoreConnect"; import { useSelector } from "./useSelector"; export type AdminRole = { @@ -8,7 +12,7 @@ export type AdminRole = { users: string[]; }; -export const adminRoleQuery: SparkleRFDocQuery = { +export const adminRoleQuery: ReduxFirestoreQuerySetting = { collection: "roles", doc: "admin", storeAs: "adminRole", diff --git a/src/hooks/useConnectCurrentVenueNG.ts b/src/hooks/useConnectCurrentVenueNG.ts index 7ddd2b1b8b..f5ea9e3dcb 100644 --- a/src/hooks/useConnectCurrentVenueNG.ts +++ b/src/hooks/useConnectCurrentVenueNG.ts @@ -7,7 +7,7 @@ import { VenueEvent } from "types/VenueEvent"; import { withId } from "utils/id"; import { useSelector } from "./useSelector"; -import { useFirestoreConnect } from "./useFirestoreConnect"; +import { useSparkleFirestoreConnect } from "./useSparkleFirestoreConnect"; export const currentVenueNGSelector: SparkleSelector = ( state @@ -18,7 +18,7 @@ export const currentVenueEventsNGSelector: SparkleSelector< > = (state) => state.firestore.data.currentVenueEventsNG; export const useConnectCurrentVenueNG = (venueId?: string) => { - useFirestoreConnect( + useSparkleFirestoreConnect( !!venueId ? [ { diff --git a/src/hooks/useConnectRelatedVenues.ts b/src/hooks/useConnectRelatedVenues.ts index ddf6a043bf..c142e8e798 100644 --- a/src/hooks/useConnectRelatedVenues.ts +++ b/src/hooks/useConnectRelatedVenues.ts @@ -24,11 +24,9 @@ import { import { useConnectCurrentVenueNG } from "./useConnectCurrentVenueNG"; import { useSelector } from "./useSelector"; import { - AnySparkleRFQuery, - SparkleRFDocQuery, - SparkleRFQuery, - useFirestoreConnect, -} from "./useFirestoreConnect"; + SparkleRFQConfig, + useSparkleFirestoreConnect, +} from "./useSparkleFirestoreConnect"; const toEventsWithVenueIds = (venueId: string) => (event: VenueEvent) => withVenueId(event, venueId); @@ -156,7 +154,7 @@ export const useConnectRelatedVenues: ReactHook< ///////////////////////////////// // Sibling - const siblingVenuesQuery: SparkleRFQuery | undefined = !!parentId + const siblingVenuesQuery: SparkleRFQConfig | undefined = !!parentId ? { collection: "venues", where: [["parentId", "==", parentId]], @@ -165,7 +163,7 @@ export const useConnectRelatedVenues: ReactHook< : undefined; // Sub - const subvenuesQuery: SparkleRFQuery | undefined = !!venueId + const subvenuesQuery: SparkleRFQConfig | undefined = !!venueId ? { collection: "venues", where: [["parentId", "==", venueId]], @@ -176,37 +174,37 @@ export const useConnectRelatedVenues: ReactHook< const makeEventsQueryConfig = ( doc: string, storeAs: string - ): SparkleRFDocQuery => + ): SparkleRFQConfig => ({ collection: "venues", doc, subcollections: [{ collection: "events" }], orderBy: ["start_utc_seconds", "asc"], storeAs, - } as SparkleRFDocQuery); // @debt a little hacky, but we're consciously subverting our helper protections; + } as SparkleRFQConfig); // @debt a little hacky, but we're consciously subverting our helper protections; // Parent Events - const parentVenueEventsQuery: SparkleRFDocQuery | undefined = + const parentVenueEventsQuery: SparkleRFQConfig | undefined = parentId && withEvents ? makeEventsQueryConfig(parentId, "parentVenueEvents") : undefined; // Sibling Events - const siblingVenueEventsQueries: SparkleRFDocQuery[] = withEvents + const siblingVenueEventsQueries: SparkleRFQConfig[] = withEvents ? siblingVenues.map((sibling) => makeEventsQueryConfig(sibling.id, `siblingVenueEvents-${sibling.id}`) ) : []; // Sub Events - const subvenueEventsQueries: SparkleRFDocQuery[] = withEvents + const subvenueEventsQueries: SparkleRFQConfig[] = withEvents ? subvenues.map((subvenue) => makeEventsQueryConfig(subvenue.id, `subvenueEvents-${subvenue.id}`) ) : []; // Combine / filter for valid queries - const allValidQueries: AnySparkleRFQuery[] = [ + const allValidQueries: SparkleRFQConfig[] = [ siblingVenuesQuery, subvenuesQuery, parentVenueEventsQuery, @@ -215,7 +213,7 @@ export const useConnectRelatedVenues: ReactHook< ].filter(isTruthyFilter); // Connect - useFirestoreConnect(allValidQueries); + useSparkleFirestoreConnect(allValidQueries); ///////////////////////////////// // Return diff --git a/src/hooks/useFirestoreConnect.ts b/src/hooks/useFirestoreConnect.ts deleted file mode 100644 index 34adf10457..0000000000 --- a/src/hooks/useFirestoreConnect.ts +++ /dev/null @@ -1,86 +0,0 @@ -import { - ReduxFirestoreQuerySetting, - useFirestoreConnect as _useFirestoreConnect, - isLoaded as _isLoaded, - isEmpty as _isEmpty, - WhereOptions, -} from "react-redux-firebase"; -import { ValidFirestoreKeys } from "types/Firestore"; -import { Defined } from "types/utility"; - -/** - * This type allows us to avoid bugs when doc is not specified, and - * automagically constrain the storeAs parameter to keys that exist - * within ValidFirestoreKeys, ensuring that we can't forget to define - * it in our types when using functions that rely on this type - * (eg. useFirestoreConnect) - * - * @see ValidFirestoreKeys - * @see useFirestoreConnect - * @see ReduxFirestoreQuerySetting - */ -export interface SparkleRFQuery extends ReduxFirestoreQuerySetting { - doc?: never; - where: WhereOptions | WhereOptions[]; - storeAs?: ValidFirestoreKeys; -} - -/** - * This type allows us to query for a single document, and automagically - * constrain the storeAs parameter to keys that exist within - * ValidFirestoreKeys, ensuring that we can't forget to define it in our - * types when using functions that rely on this type - * (eg. useFirestoreConnect) - * - * @see AnySparkleRFQuery - * @see useFirestoreConnect - * @see ReduxFirestoreQuerySetting - */ -export interface SparkleRFDocQuery extends ReduxFirestoreQuerySetting { - doc: string; - where?: never; - storeAs?: ValidFirestoreKeys; -} - -export type AnySparkleRFQuery = SparkleRFQuery | SparkleRFDocQuery; - -/** - * A wrapper for useFirestoreConnect() that ensures the config - * we pass it can only use a storeAs key that has been defined - * in our FirestoreData/FirestoreOrdered types. - * - * Note: the config does NOT need to be memo'd before being passed - * to this function as useFirestoreConnect() determines equality - * through a deep comparison using lodash's isEqual() function. - * - * @param config the config to be passed to useFirestoreConnect() - * @param shouldExecuteQuery if the value passed is falsy, - * don't execute the query (ie. no-op). - * - * @see AnySparkleRFQuery - * @see ValidFirestoreKeys - * @see ReduxFirestoreQuerySetting - */ -export const useFirestoreConnect = ( - config: AnySparkleRFQuery | AnySparkleRFQuery[], - shouldExecuteQuery?: boolean -) => { - if (!shouldExecuteQuery) return; - - return _useFirestoreConnect(config); -}; - -/** - * Use react-redux-firestore's isEmpty helper with - * user-defined type guards to properly narrow types - * when using this helper. - * - * @param item item fetched by react-redux-firestore - */ -export const hasData = (item: T): item is Defined => - !_isEmpty(item) && item !== undefined; - -/** - * Re-export react-redux-firestore's isLoaded helper for convenience. - */ -export const isLoaded = (item: T) => _isLoaded(item); diff --git a/src/hooks/useSparkleFirestoreConnect.ts b/src/hooks/useSparkleFirestoreConnect.ts new file mode 100644 index 0000000000..547615dac1 --- /dev/null +++ b/src/hooks/useSparkleFirestoreConnect.ts @@ -0,0 +1,59 @@ +import { + ReduxFirestoreQuerySetting, + useFirestoreConnect, + isLoaded as _isLoaded, + isEmpty as _isEmpty, +} from "react-redux-firebase"; +import { ValidFirestoreKeys } from "types/Firestore"; + +/** + * Type helper representing all types of T except undefined + */ +export type Defined = T & Exclude; + +/** + * This type allows us to automagically constrain the storeAs + * parameter to keys that exist within ValidFirestoreKeys, ensuring + * that we can't forget to define it in our types when using functions + * that rely on this type (eg. useSparkleFirestoreConnect) + * + * @see ValidFirestoreKeys + * @see useSparkleFirestoreConnect + * @see ReduxFirestoreQuerySetting + */ +export interface SparkleRFQConfig extends ReduxFirestoreQuerySetting { + storeAs?: ValidFirestoreKeys; +} + +/** + * A wrapper for useFirestoreConnect() that ensures the config + * we pass it can only use a storeAs key that has been defined + * in our FirestoreData/FirestoreOrdered types. + * + * Note: the config does NOT need to be memo'd before being passed + * to this function as useSparkleFirestoreConnect() determines equality + * through a deep comparison using lodash's isEqual() function. + * + * @param config the config to be passed to useFirestoreConnect() + * + * @see SparkleRFQConfig + * @see ValidFirestoreKeys + * @see ReduxFirestoreQuerySetting + */ +export const useSparkleFirestoreConnect = (config: SparkleRFQConfig[]) => + useFirestoreConnect(config); + +/** + * Use react-redux-firestore's isEmpty helper with + * user-defined type guards to properly narrow types + * when using this helper. + * + * @param item item fetched by react-redux-firestore + */ +export const hasData = (item: T): item is Defined => + !_isEmpty(item) && item !== undefined; + +/** + * Re-export react-redux-firestore's isLoaded helper for convenience. + */ +export const isLoaded = (item: T) => _isLoaded(item); diff --git a/src/pages/Admin/Admin.tsx b/src/pages/Admin/Admin.tsx index 74e8743037..1bc5e4fcde 100644 --- a/src/pages/Admin/Admin.tsx +++ b/src/pages/Admin/Admin.tsx @@ -15,6 +15,10 @@ import { useRouteMatch, useHistory, } from "react-router-dom"; +import { + ReduxFirestoreQuerySetting, + useFirestoreConnect, +} from "react-redux-firebase"; import dayjs from "dayjs"; import advancedFormat from "dayjs/plugin/advancedFormat"; import "firebase/storage"; @@ -63,7 +67,6 @@ import VenueDeleteModal from "./Venue/VenueDeleteModal"; import { VenueOwnersModal } from "./VenueOwnersModal"; import "./Admin.scss"; -import { SparkleRFQuery, useFirestoreConnect } from "hooks/useFirestoreConnect"; dayjs.extend(advancedFormat); @@ -427,7 +430,7 @@ const Admin: React.FC = () => { const { isAdminUser, isLoading: isAdminUserLoading } = useIsAdminUser(userId); - const venuesOwnedByUserQuery = useMemo( + const venuesOwnedByUserQuery = useMemo( () => ({ collection: "venues", where: [["owners", "array-contains", userId]], diff --git a/src/types/utility.ts b/src/types/utility.ts index 1ba075466a..00ef1bc4cb 100644 --- a/src/types/utility.ts +++ b/src/types/utility.ts @@ -1,8 +1,3 @@ -/** - * Type helper representing all types of T except undefined - */ -export type Defined = T & Exclude; - export type ExtractProps = T extends React.FunctionComponent ? U : never;