From d5f109ac4e5fbc3c90ca61657dd2a820d155720b Mon Sep 17 00:00:00 2001 From: Oliver Eyton-Williams Date: Fri, 9 Aug 2024 15:40:06 +0200 Subject: [PATCH] fix(client): only fetch completion data on challenge pages (#55787) --- client/src/components/Progress/progress.tsx | 71 +++++++++++++++++-- client/src/components/layouts/default.tsx | 63 ++-------------- client/src/redux/index.js | 2 +- client/src/redux/prop-types.ts | 2 +- .../components/completion-modal.tsx | 10 +-- .../Challenges/redux/completion-epic.test.js | 2 +- client/src/utils/get-completion-percentage.ts | 8 +-- 7 files changed, 78 insertions(+), 80 deletions(-) diff --git a/client/src/components/Progress/progress.tsx b/client/src/components/Progress/progress.tsx index 62b52a83fef..23935003f8c 100644 --- a/client/src/components/Progress/progress.tsx +++ b/client/src/components/Progress/progress.tsx @@ -1,8 +1,10 @@ -import React from 'react'; -import { connect } from 'react-redux'; +import React, { useEffect } from 'react'; +import { connect, ConnectedProps } from 'react-redux'; import { createSelector } from 'reselect'; import { TFunction } from 'i18next'; import { withTranslation } from 'react-i18next'; +import { useStaticQuery, graphql } from 'gatsby'; + import { challengeMetaSelector, currentBlockIdsSelector, @@ -10,6 +12,8 @@ import { completedPercentageSelector } from '../../templates/Challenges/redux/selectors'; import { liveCerts } from '../../../config/cert-and-project-map'; +import { updateAllChallengesInfo } from '../../redux/actions'; +import { CertificateNode, ChallengeNode } from '../../redux/prop-types'; import ProgressInner from './progress-inner'; const mapStateToProps = createSelector( @@ -40,9 +44,11 @@ const mapStateToProps = createSelector( }) ); -type StateProps = ReturnType; +const mapDispatchToProps = { updateAllChallengesInfo }; -interface ProgressProps extends StateProps { +type PropsFromRedux = ConnectedProps; + +interface ProgressProps extends PropsFromRedux { t: TFunction; } function Progress({ @@ -52,7 +58,8 @@ function Progress({ superBlock, completedChallengesInBlock, completedPercent, - t + t, + updateAllChallengesInfo }: ProgressProps): JSX.Element { const blockTitle = t(`intro:${superBlock}.blocks.${block}.title`); // Always false for legacy full stack, since it has no projects. @@ -60,6 +67,11 @@ function Progress({ cert.projects?.some((project: { id: string }) => project.id === id) ); + const { challengeNodes, certificateNodes } = useGetAllBlockIds(); + useEffect(() => { + updateAllChallengesInfo({ challengeNodes, certificateNodes }); + }, [challengeNodes, certificateNodes, updateAllChallengesInfo]); + const totalChallengesInBlock = currentBlockIds?.length ?? 0; const meta = isCertificationProject && totalChallengesInBlock > 0 @@ -84,6 +96,53 @@ function Progress({ ); } +// TODO: extract this hook and call it when needed (i.e. here, in the lower-jaw +// and in completion-modal). Then we don't have to pass the data into redux. +// This would mean that we have to memoize any complex calculations in the hook. +// Otherwise, this will undo all the recent performance improvements. +const useGetAllBlockIds = () => { + const { + allChallengeNode: { nodes: challengeNodes }, + allCertificateNode: { nodes: certificateNodes } + }: { + allChallengeNode: { nodes: ChallengeNode[] }; + allCertificateNode: { nodes: CertificateNode[] }; + } = useStaticQuery(graphql` + query getBlockNode { + allChallengeNode( + sort: { + fields: [ + challenge___superOrder + challenge___order + challenge___challengeOrder + ] + } + ) { + nodes { + challenge { + block + id + } + } + } + allCertificateNode { + nodes { + challenge { + certification + tests { + id + } + } + } + } + } + `); + + return { challengeNodes, certificateNodes }; +}; + Progress.displayName = 'Progress'; -export default connect(mapStateToProps)(withTranslation()(Progress)); +const connector = connect(mapStateToProps, mapDispatchToProps); + +export default connector(withTranslation()(Progress)); diff --git a/client/src/components/layouts/default.tsx b/client/src/components/layouts/default.tsx index 1958b0c7c91..88754c42c1f 100644 --- a/client/src/components/layouts/default.tsx +++ b/client/src/components/layouts/default.tsx @@ -5,7 +5,6 @@ import { useMediaQuery } from 'react-responsive'; import { connect } from 'react-redux'; import { bindActionCreators, Dispatch } from 'redux'; import { createSelector } from 'reselect'; -import { useStaticQuery, graphql } from 'gatsby'; import latoBoldURL from '../../../static/fonts/lato/Lato-Bold.woff'; import latoLightURL from '../../../static/fonts/lato/Lato-Light.woff'; @@ -18,8 +17,7 @@ import { isBrowser } from '../../../utils'; import { fetchUser, onlineStatusChange, - serverStatusChange, - updateAllChallengesInfo + serverStatusChange } from '../../redux/actions'; import { isSignedInSelector, @@ -30,12 +28,7 @@ import { userFetchStateSelector } from '../../redux/selectors'; -import { - UserFetchState, - User, - AllChallengeNode, - CertificateNode -} from '../../redux/prop-types'; +import { UserFetchState, User } from '../../redux/prop-types'; import BreadCrumb from '../../templates/Challenges/components/bread-crumb'; import Flash from '../Flash'; import { flashMessageSelector, removeFlashMessage } from '../Flash/redux'; @@ -95,8 +88,7 @@ const mapDispatchToProps = (dispatch: Dispatch) => fetchUser, removeFlashMessage, onlineStatusChange, - serverStatusChange, - updateAllChallengesInfo + serverStatusChange }, dispatch ); @@ -138,8 +130,7 @@ function DefaultLayout({ superBlock, theme, user, - fetchUser, - updateAllChallengesInfo + fetchUser }: DefaultLayoutProps): JSX.Element { const { t } = useTranslation(); const isMobileLayout = useMediaQuery({ maxWidth: MAX_MOBILE_WIDTH }); @@ -150,10 +141,8 @@ function DefaultLayout({ const isExSmallViewportHeight = useMediaQuery({ maxHeight: EX_SMALL_VIEWPORT_HEIGHT }); - const { challengeEdges, certificateNodes } = useGetAllBlockIds(); useEffect(() => { // componentDidMount - updateAllChallengesInfo({ challengeEdges, certificateNodes }); if (!isSignedIn) { fetchUser(); } @@ -278,50 +267,6 @@ function DefaultLayout({ } } -// TODO: get challenge nodes directly rather than wrapped in edges -const useGetAllBlockIds = () => { - const { - allChallengeNode: { edges: challengeEdges }, - allCertificateNode: { nodes: certificateNodes } - }: { - allChallengeNode: AllChallengeNode; - allCertificateNode: { nodes: CertificateNode[] }; - } = useStaticQuery(graphql` - query getBlockNode { - allChallengeNode( - sort: { - fields: [ - challenge___superOrder - challenge___order - challenge___challengeOrder - ] - } - ) { - edges { - node { - challenge { - block - id - } - } - } - } - allCertificateNode { - nodes { - challenge { - certification - tests { - id - } - } - } - } - } - `); - - return { challengeEdges, certificateNodes }; -}; - DefaultLayout.displayName = 'DefaultLayout'; export default connect( diff --git a/client/src/redux/index.js b/client/src/redux/index.js index 7cde5df3991..2720b42186c 100644 --- a/client/src/redux/index.js +++ b/client/src/redux/index.js @@ -66,7 +66,7 @@ const initialState = { ...defaultFetchState }, allChallengesInfo: { - challengeEdges: [], + challengeNodes: [], certificateNodes: [] }, userProfileFetchState: { diff --git a/client/src/redux/prop-types.ts b/client/src/redux/prop-types.ts index 7a5969f0c52..72b5f053ed1 100644 --- a/client/src/redux/prop-types.ts +++ b/client/src/redux/prop-types.ts @@ -243,7 +243,7 @@ export type CertificateNode = { }; export type AllChallengesInfo = { - challengeEdges: { node: ChallengeNode }[]; + challengeNodes: ChallengeNode[]; certificateNodes: CertificateNode[]; }; diff --git a/client/src/templates/Challenges/components/completion-modal.tsx b/client/src/templates/Challenges/components/completion-modal.tsx index 90e4b95aaf1..251b9da65eb 100644 --- a/client/src/templates/Challenges/components/completion-modal.tsx +++ b/client/src/templates/Challenges/components/completion-modal.tsx @@ -8,11 +8,8 @@ import { createSelector } from 'reselect'; import { Button, Modal } from '@freecodecamp/ui'; import Login from '../../../components/Header/components/login'; -import { - isSignedInSelector, - allChallengesInfoSelector -} from '../../../redux/selectors'; -import { AllChallengesInfo, ChallengeFiles } from '../../../redux/prop-types'; +import { isSignedInSelector } from '../../../redux/selectors'; +import { ChallengeFiles } from '../../../redux/prop-types'; import { closeModal, submitChallenge } from '../redux/actions'; import { completedChallengesIdsSelector, @@ -35,7 +32,6 @@ const mapStateToProps = createSelector( completedChallengesIdsSelector, isCompletionModalOpenSelector, isSignedInSelector, - allChallengesInfoSelector, successMessageSelector, isSubmittingSelector, ( @@ -44,7 +40,6 @@ const mapStateToProps = createSelector( completedChallengesIds: string[], isOpen: boolean, isSignedIn: boolean, - allChallengesInfo: AllChallengesInfo, message: string, isSubmitting: boolean ) => ({ @@ -54,7 +49,6 @@ const mapStateToProps = createSelector( completedChallengesIds, isOpen, isSignedIn, - allChallengesInfo, message, isSubmitting }) diff --git a/client/src/templates/Challenges/redux/completion-epic.test.js b/client/src/templates/Challenges/redux/completion-epic.test.js index c1f62159230..3d15e634ee4 100644 --- a/client/src/templates/Challenges/redux/completion-epic.test.js +++ b/client/src/templates/Challenges/redux/completion-epic.test.js @@ -19,7 +19,7 @@ describe('completionEpic', () => { challenge: { challengeMeta: { challengeType: 0 } }, app: { user: { username: 'test' }, - allChallengesInfo: { challengeEdges: [], certificateNodes: [] } + allChallengesInfo: { challengeNodes: [], certificateNodes: [] } } } }; diff --git a/client/src/utils/get-completion-percentage.ts b/client/src/utils/get-completion-percentage.ts index fc1516f46e7..96a0dbe1a6d 100644 --- a/client/src/utils/get-completion-percentage.ts +++ b/client/src/utils/get-completion-percentage.ts @@ -39,14 +39,14 @@ export const getCurrentBlockIds = ( certification: string, challengeType: number ): string[] => { - const { challengeEdges, certificateNodes } = allChallengesInfo; + const { challengeNodes, certificateNodes } = allChallengesInfo; const currentCertificateIds = certificateNodes .filter(node => node.challenge.certification === certification)[0] ?.challenge.tests.map(test => test.id) ?? []; - const currentBlockIds = challengeEdges - .filter(edge => edge.node.challenge.block === block) - .map(edge => edge.node.challenge.id); + const currentBlockIds = challengeNodes + .filter(node => node.challenge.block === block) + .map(node => node.challenge.id); return isProjectBased(challengeType) ? currentCertificateIds