From 128c48d1f2a05c80b4f385929b5b2d178d17d2a8 Mon Sep 17 00:00:00 2001 From: pieperm Date: Sun, 8 Aug 2021 23:39:15 -0500 Subject: [PATCH 1/5] Access query params asynchronously --- web-ui/src/pages/FeedbackRequestPage.jsx | 90 +++++++++++++----------- 1 file changed, 48 insertions(+), 42 deletions(-) diff --git a/web-ui/src/pages/FeedbackRequestPage.jsx b/web-ui/src/pages/FeedbackRequestPage.jsx index e8f840b24e..40843dd474 100644 --- a/web-ui/src/pages/FeedbackRequestPage.jsx +++ b/web-ui/src/pages/FeedbackRequestPage.jsx @@ -1,4 +1,4 @@ -import React, {useContext, useCallback, useEffect, useState} from "react"; +import React, {useContext, useCallback, useEffect, useState, useRef} from "react"; import { makeStyles } from "@material-ui/core/styles"; import Stepper from "@material-ui/core/Stepper"; import Step from "@material-ui/core/Step"; @@ -78,23 +78,12 @@ const FeedbackRequestPage = () => { const currentUserId = memberProfile?.id; const location = useLocation(); const history = useHistory(); - const [query, setQuery] = useState({}); - const stepQuery = query.step?.toString(); - const templateQuery = query.template?.toString(); - const fromQuery = query.from ? query.from : []; - const sendQuery = query.send?.toString(); - const dueQuery = query.due?.toString(); - const sendDate = query.send?.toString(); - const forQuery = query.for?.toString(); - const requestee = selectProfile(state, forQuery); const memberIds = selectCurrentMemberIds(state); const csrf = selectCsrfToken(state); + const [query, setQuery] = useState({}); const [readyToProceed, setReadyToProceed] = useState(false); const [templateIsValid, setTemplateIsValid] = useState(); - - useEffect(() => { - setQuery(queryString.parse(location?.search)); - }, [location.search]); + const [requestee, setRequestee] = useState({}); const handleQueryChange = useCallback((key, value) => { let newQuery = { @@ -105,23 +94,23 @@ const FeedbackRequestPage = () => { }, [history, location, query]); const getStep = useCallback(() => { - if (!stepQuery || stepQuery < 1 || !/^\d+$/.test(stepQuery)) + if (!query.step || query.step < 1 || !/^\d+$/.test(query.step)) return 1; - else return parseInt(stepQuery); - }, [stepQuery]); + else return parseInt(query.step); + }, [query.step]); const activeStep = getStep(); const hasFor = useCallback(() => { - return !!forQuery; - }, [forQuery]) + return !!query.for; + }, [query.for]); useEffect(() => { - async function isTemplateValid() { - if (!templateQuery || !csrf) { + async function isTemplateValid() { + if (!query.template || !csrf) { return false; } - let res = await getFeedbackTemplate(templateQuery, csrf); + let res = await getFeedbackTemplate(query.template, csrf); let templateResponse = res.payload && res.payload.data && @@ -147,7 +136,7 @@ const FeedbackRequestPage = () => { isTemplateValid().then((isValid) => { setTemplateIsValid(isValid); }); - }, [csrf, templateQuery]); + }, [csrf, query.template]); const hasFrom = useCallback(() => { let from = query.from; @@ -182,38 +171,38 @@ const FeedbackRequestPage = () => { }, []); const hasSend = useCallback(() => { - const isValidPair = dueQuery ? dueQuery >= sendQuery : true; - return (sendQuery && isValidDate(sendQuery) && isValidPair) - }, [sendQuery, isValidDate, dueQuery]); + const isValidPair = query.due ? query.due >= query.send : true; + return (query.send && isValidDate(query.send) && isValidPair) + }, [query.send, isValidDate, query.due]); const canProceed = useCallback(() => { if (query && Object.keys(query).length > 0) { switch (activeStep) { case 1: - return hasFor() && templateIsValid + return hasFor() && templateIsValid; case 2: return hasFor() && templateIsValid && hasFrom(); case 3: - const dueQueryValid = dueQuery ? isValidDate(dueQuery) : true; + const dueQueryValid = query.due ? isValidDate(query.due) : true; return hasFor() && templateIsValid && hasFrom() && hasSend() && dueQueryValid; default: return false; } } return false; - }, [activeStep, hasFor, hasFrom, hasSend, dueQuery, isValidDate, query, templateIsValid]); + }, [activeStep, hasFor, hasFrom, hasSend, isValidDate, query, templateIsValid]); const handleSubmit = () => { - const from = fromQuery ? (Array.isArray(fromQuery) ? fromQuery : [fromQuery]) : []; + const from = query.from ? (Array.isArray(query.from) ? query.from : [query.from]) : []; for (const recipient of from) { const feedbackRequest = { id: null, creatorId: currentUserId, - requesteeId: forQuery, + requesteeId: query.for, recipientId: recipient, - templateId: templateQuery, - sendDate: sendDate, - dueDate: dueQuery, + templateId: query.template, + sendDate: query.send, + dueDate: query.due, status: "pending", submitDate: null }; @@ -272,12 +261,12 @@ const FeedbackRequestPage = () => { } const urlIsValid = useCallback(() => { - if (query && Object.keys(query).length > 0) { + if (query) { switch (activeStep) { case 1: return hasFor(); case 2: - return hasFor() && templateIsValid + return hasFor() && templateIsValid; case 3: return hasFor() && templateIsValid && hasFrom(); case 4: @@ -290,6 +279,23 @@ const FeedbackRequestPage = () => { }, [activeStep, hasFor, hasFrom, hasSend, query, templateIsValid]); useEffect(() => { + queryLoaded.current = false; + const params = queryString.parse(location?.search); + console.log(params); + console.log(query); + setQuery(params); + }, [location.search]); + + useEffect(() => { + console.log(query); + }, [query]); + + useEffect(() => { + console.log(query); + queryLoaded.current = true; + if (query.for) { + setRequestee(selectProfile(state, query.for)); + } if (!urlIsValid()) { dispatch({ type: UPDATE_TOAST, @@ -302,11 +308,11 @@ const FeedbackRequestPage = () => { history.push("/checkins"); }); } - }, [history, urlIsValid, dispatch, softDeleteAdHoc, currentUserId]); + }, [state, query, currentUserId, dispatch, softDeleteAdHoc, urlIsValid]); - useEffect(()=> { + useEffect(() => { setReadyToProceed(canProceed()); - }, [canProceed]) + }, [canProceed]); return (
@@ -337,9 +343,9 @@ const FeedbackRequestPage = () => {
- {activeStep === 1 && handleQueryChange(key, value)} query={templateQuery} />} - {activeStep === 2 && handleQueryChange(key, value)} fromQuery={Array.isArray(fromQuery) ? fromQuery : [fromQuery]} />} - {activeStep === 3 && handleQueryChange(key, value)} sendDateQuery={sendQuery} dueDateQuery={dueQuery}/>} + {activeStep === 1 && handleQueryChange(key, value)} query={query.template} />} + {activeStep === 2 && handleQueryChange(key, value)} fromQuery={query.from ? (Array.isArray(query.from) ? query.from : [query.from]) : []} />} + {activeStep === 3 && handleQueryChange(key, value)} sendDateQuery={query.send} dueDateQuery={query.due}/>}
); From 75838936de55e37b9d2c24ee624aa7de6050029b Mon Sep 17 00:00:00 2001 From: pieperm Date: Sun, 8 Aug 2021 23:39:32 -0500 Subject: [PATCH 2/5] Access query params asynchronously --- web-ui/src/pages/FeedbackRequestPage.jsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/web-ui/src/pages/FeedbackRequestPage.jsx b/web-ui/src/pages/FeedbackRequestPage.jsx index 40843dd474..9f2055a88b 100644 --- a/web-ui/src/pages/FeedbackRequestPage.jsx +++ b/web-ui/src/pages/FeedbackRequestPage.jsx @@ -279,7 +279,6 @@ const FeedbackRequestPage = () => { }, [activeStep, hasFor, hasFrom, hasSend, query, templateIsValid]); useEffect(() => { - queryLoaded.current = false; const params = queryString.parse(location?.search); console.log(params); console.log(query); @@ -292,7 +291,6 @@ const FeedbackRequestPage = () => { useEffect(() => { console.log(query); - queryLoaded.current = true; if (query.for) { setRequestee(selectProfile(state, query.for)); } From cfc1c20cf6d9c22b6818afd9ce672aac1ec81baf Mon Sep 17 00:00:00 2001 From: pieperm Date: Mon, 9 Aug 2021 12:41:13 -0500 Subject: [PATCH 3/5] Fix URL validation being too strict --- web-ui/src/pages/FeedbackRequestPage.jsx | 155 ++++++++++++----------- 1 file changed, 81 insertions(+), 74 deletions(-) diff --git a/web-ui/src/pages/FeedbackRequestPage.jsx b/web-ui/src/pages/FeedbackRequestPage.jsx index 9f2055a88b..d5aa0d2cd1 100644 --- a/web-ui/src/pages/FeedbackRequestPage.jsx +++ b/web-ui/src/pages/FeedbackRequestPage.jsx @@ -78,12 +78,13 @@ const FeedbackRequestPage = () => { const currentUserId = memberProfile?.id; const location = useLocation(); const history = useHistory(); - const memberIds = selectCurrentMemberIds(state); const csrf = selectCsrfToken(state); const [query, setQuery] = useState({}); + const queryLoaded = useRef(false); const [readyToProceed, setReadyToProceed] = useState(false); const [templateIsValid, setTemplateIsValid] = useState(); const [requestee, setRequestee] = useState({}); + const [memberIds, setMemberIds] = useState([]); const handleQueryChange = useCallback((key, value) => { let newQuery = { @@ -99,46 +100,12 @@ const FeedbackRequestPage = () => { else return parseInt(query.step); }, [query.step]); - const activeStep = getStep(); - const hasFor = useCallback(() => { return !!query.for; }, [query.for]); - useEffect(() => { - async function isTemplateValid() { - if (!query.template || !csrf) { - return false; - } - let res = await getFeedbackTemplate(query.template, csrf); - let templateResponse = - res.payload && - res.payload.data && - res.payload.status === 200 && - !res.error - ? res.payload.data - : null - if (templateResponse === null) { - window.snackDispatch({ - type: UPDATE_TOAST, - payload: { - severity: "error", - toast: "The ID for the template you selected does not exist.", - }, - }); - return false; - } - else { - return true; - } - } - - isTemplateValid().then((isValid) => { - setTemplateIsValid(isValid); - }); - }, [csrf, query.template]); - const hasFrom = useCallback(() => { + if (!memberIds.length) return true; let from = query.from; if (from) { from = Array.isArray(from) ? from : [from]; @@ -177,20 +144,20 @@ const FeedbackRequestPage = () => { const canProceed = useCallback(() => { if (query && Object.keys(query).length > 0) { - switch (activeStep) { - case 1: - return hasFor() && templateIsValid; - case 2: - return hasFor() && templateIsValid && hasFrom(); - case 3: - const dueQueryValid = query.due ? isValidDate(query.due) : true; - return hasFor() && templateIsValid && hasFrom() && hasSend() && dueQueryValid; - default: - return false; + const activeStep = getStep(); + if (activeStep === 1) { + return hasFor() && templateIsValid; + } else if (activeStep === 2) { + return hasFor() && templateIsValid && hasFrom(); + } else if (activeStep === 3) { + const dueQueryValid = query.due ? isValidDate(query.due) : true; + return hasFor() && templateIsValid && hasFrom() && hasSend() && dueQueryValid; + } else { + return false; } } return false; - }, [activeStep, hasFor, hasFrom, hasSend, isValidDate, query, templateIsValid]); + }, [hasFor, hasFrom, hasSend, isValidDate, query, templateIsValid, getStep]); const handleSubmit = () => { const from = query.from ? (Array.isArray(query.from) ? query.from : [query.from]) : []; @@ -212,6 +179,7 @@ const FeedbackRequestPage = () => { const onNextClick = useCallback(() => { if (!canProceed()) return; + const activeStep = getStep(); if (activeStep === steps.length) { handleSubmit(); return; @@ -219,13 +187,14 @@ const FeedbackRequestPage = () => { query.step = `${activeStep + 1}`; history.push({...location, search: queryString.stringify(query)}); - }, [canProceed, activeStep, steps.length, query, location, history]); // eslint-disable-line react-hooks/exhaustive-deps + }, [canProceed, steps.length, query, location, history]); // eslint-disable-line react-hooks/exhaustive-deps const onBackClick = useCallback(() => { + const activeStep = getStep(); if (activeStep === 1) return; query.step = `${activeStep - 1}`; history.push({ ...location, search: queryString.stringify(query) }); - }, [activeStep, query, location, history]); + }, [query, location, history, getStep]); const softDeleteAdHoc = useCallback(async (creatorId) => { if (csrf) { @@ -262,35 +231,72 @@ const FeedbackRequestPage = () => { const urlIsValid = useCallback(() => { if (query) { - switch (activeStep) { - case 1: - return hasFor(); - case 2: - return hasFor() && templateIsValid; - case 3: - return hasFor() && templateIsValid && hasFrom(); - case 4: - return hasFor() && templateIsValid && hasFrom() && hasSend(); - default: - return false; + const activeStep = getStep(); + if (activeStep === 1) { + return hasFor(); + } else if (activeStep === 2) { + return hasFor() && templateIsValid; + } else if (activeStep === 3) { + return hasFor() && templateIsValid && hasFrom(); + } else { + return false; } } - return true; - }, [activeStep, hasFor, hasFrom, hasSend, query, templateIsValid]); + return false; + }, [hasFor, hasFrom, query, templateIsValid, getStep]); + + useEffect(() => { + const members = selectCurrentMemberIds(state); + if (members) { + setMemberIds(members); + } + }, [state]); useEffect(() => { const params = queryString.parse(location?.search); - console.log(params); - console.log(query); setQuery(params); }, [location.search]); useEffect(() => { - console.log(query); - }, [query]); + async function isTemplateValid() { + if (!query.template || !csrf) { + return false; + } + let res = await getFeedbackTemplate(query.template, csrf); + let templateResponse = + res.payload && + res.payload.data && + res.payload.status === 200 && + !res.error + ? res.payload.data + : null + if (templateResponse === null) { + window.snackDispatch({ + type: UPDATE_TOAST, + payload: { + severity: "error", + toast: "The ID for the template you selected does not exist.", + }, + }); + return false; + } + else { + return true; + } + } + + if (queryLoaded.current && csrf) { + isTemplateValid().then((isValid) => { + setTemplateIsValid(isValid); + }); + } else { + queryLoaded.current = true; + } + }, [csrf, query, queryLoaded]); useEffect(() => { - console.log(query); + if (!queryLoaded.current || templateIsValid === undefined) return; + if (query.for) { setRequestee(selectProfile(state, query.for)); } @@ -306,29 +312,30 @@ const FeedbackRequestPage = () => { history.push("/checkins"); }); } - }, [state, query, currentUserId, dispatch, softDeleteAdHoc, urlIsValid]); + }, [history, state, query, currentUserId, dispatch, softDeleteAdHoc, urlIsValid, templateIsValid]); useEffect(() => { setReadyToProceed(canProceed()); }, [canProceed]); return ( + queryLoaded.current &&
Feedback Request for {requestee?.name}
-
- + {steps.map((label) => { const stepProps = {}; const labelProps = {}; @@ -341,9 +348,9 @@ const FeedbackRequestPage = () => {
- {activeStep === 1 && handleQueryChange(key, value)} query={query.template} />} - {activeStep === 2 && handleQueryChange(key, value)} fromQuery={query.from ? (Array.isArray(query.from) ? query.from : [query.from]) : []} />} - {activeStep === 3 && handleQueryChange(key, value)} sendDateQuery={query.send} dueDateQuery={query.due}/>} + {getStep() === 1 && handleQueryChange(key, value)} query={query.template} />} + {getStep() === 2 && handleQueryChange(key, value)} fromQuery={query.from ? (Array.isArray(query.from) ? query.from : [query.from]) : []} />} + {getStep() === 3 && handleQueryChange(key, value)} sendDateQuery={query.send} dueDateQuery={query.due}/>}
); From b85e9b9bab98327724dbe65e8d892cecae702a32 Mon Sep 17 00:00:00 2001 From: pieperm Date: Mon, 9 Aug 2021 13:31:22 -0500 Subject: [PATCH 4/5] Validate that the requestee is a member, make active step stateful --- web-ui/src/pages/FeedbackRequestPage.jsx | 86 ++++++++++++------------ 1 file changed, 44 insertions(+), 42 deletions(-) diff --git a/web-ui/src/pages/FeedbackRequestPage.jsx b/web-ui/src/pages/FeedbackRequestPage.jsx index d5aa0d2cd1..2c8cce9d50 100644 --- a/web-ui/src/pages/FeedbackRequestPage.jsx +++ b/web-ui/src/pages/FeedbackRequestPage.jsx @@ -85,6 +85,7 @@ const FeedbackRequestPage = () => { const [templateIsValid, setTemplateIsValid] = useState(); const [requestee, setRequestee] = useState({}); const [memberIds, setMemberIds] = useState([]); + const [activeStep, setActiveStep] = useState(1); const handleQueryChange = useCallback((key, value) => { let newQuery = { @@ -101,8 +102,9 @@ const FeedbackRequestPage = () => { }, [query.step]); const hasFor = useCallback(() => { - return !!query.for; - }, [query.for]); + if (!memberIds.length) return true; + return !!query.for && memberIds.includes(query.for); + }, [query.for, memberIds]); const hasFrom = useCallback(() => { if (!memberIds.length) return true; @@ -144,7 +146,6 @@ const FeedbackRequestPage = () => { const canProceed = useCallback(() => { if (query && Object.keys(query).length > 0) { - const activeStep = getStep(); if (activeStep === 1) { return hasFor() && templateIsValid; } else if (activeStep === 2) { @@ -157,7 +158,34 @@ const FeedbackRequestPage = () => { } } return false; - }, [hasFor, hasFrom, hasSend, isValidDate, query, templateIsValid, getStep]); + }, [hasFor, hasFrom, hasSend, isValidDate, query, templateIsValid, activeStep]); + + const sendFeedbackRequest = async (feedbackRequest) => { + if (csrf) { + let res = await createFeedbackRequest(feedbackRequest, csrf); + let data = + res.payload && res.payload.data && !res.error + ? res.payload.data + : null; + if (data) { + // If the request was successful, set created ad-hoc templates to inactive + await softDeleteAdHoc(currentUserId); + const newLocation = { + pathname: "/feedback/request/confirmation", + search: queryString.stringify(query), + } + history.push(newLocation) + } else if (res.error || data === null) { + dispatch({ + type: UPDATE_TOAST, + payload: { + severity: "error", + toast: "An error has occurred while submitting your request.", + }, + }); + } + } + } const handleSubmit = () => { const from = query.from ? (Array.isArray(query.from) ? query.from : [query.from]) : []; @@ -179,7 +207,6 @@ const FeedbackRequestPage = () => { const onNextClick = useCallback(() => { if (!canProceed()) return; - const activeStep = getStep(); if (activeStep === steps.length) { handleSubmit(); return; @@ -190,11 +217,10 @@ const FeedbackRequestPage = () => { }, [canProceed, steps.length, query, location, history]); // eslint-disable-line react-hooks/exhaustive-deps const onBackClick = useCallback(() => { - const activeStep = getStep(); if (activeStep === 1) return; query.step = `${activeStep - 1}`; history.push({ ...location, search: queryString.stringify(query) }); - }, [query, location, history, getStep]); + }, [query, location, history, activeStep]); const softDeleteAdHoc = useCallback(async (creatorId) => { if (csrf) { @@ -202,36 +228,8 @@ const FeedbackRequestPage = () => { } }, [csrf]); - const sendFeedbackRequest = async (feedbackRequest) => { - if (csrf) { - let res = await createFeedbackRequest(feedbackRequest, csrf); - let data = - res.payload && res.payload.data && !res.error - ? res.payload.data - : null; - if (data) { - // If the request was successful, set created ad-hoc templates to inactive - await softDeleteAdHoc(currentUserId); - const newLocation = { - pathname: "/feedback/request/confirmation", - search: queryString.stringify(query), - } - history.push(newLocation) - } else if(res.error || data === null) { - dispatch({ - type: UPDATE_TOAST, - payload: { - severity: "error", - toast: "An error has occurred while submitting your request.", - }, - }); - } - } - } - const urlIsValid = useCallback(() => { if (query) { - const activeStep = getStep(); if (activeStep === 1) { return hasFor(); } else if (activeStep === 2) { @@ -243,7 +241,11 @@ const FeedbackRequestPage = () => { } } return false; - }, [hasFor, hasFrom, query, templateIsValid, getStep]); + }, [hasFor, hasFrom, query, templateIsValid, activeStep]); + + useEffect(() => { + setActiveStep(getStep()); + }, [query.step, getStep]); useEffect(() => { const members = selectCurrentMemberIds(state); @@ -324,18 +326,18 @@ const FeedbackRequestPage = () => {
Feedback Request for {requestee?.name}
-
- + {steps.map((label) => { const stepProps = {}; const labelProps = {}; @@ -348,9 +350,9 @@ const FeedbackRequestPage = () => {
- {getStep() === 1 && handleQueryChange(key, value)} query={query.template} />} - {getStep() === 2 && handleQueryChange(key, value)} fromQuery={query.from ? (Array.isArray(query.from) ? query.from : [query.from]) : []} />} - {getStep() === 3 && handleQueryChange(key, value)} sendDateQuery={query.send} dueDateQuery={query.due}/>} + {activeStep === 1 && handleQueryChange(key, value)} query={query.template} />} + {activeStep === 2 && handleQueryChange(key, value)} fromQuery={query.from ? (Array.isArray(query.from) ? query.from : [query.from]) : []} />} + {activeStep === 3 && handleQueryChange(key, value)} sendDateQuery={query.send} dueDateQuery={query.due}/>}
); From 0126b130748ea64e7d4e78b41beb8c3d2df7b422 Mon Sep 17 00:00:00 2001 From: pieperm Date: Mon, 9 Aug 2021 13:35:04 -0500 Subject: [PATCH 5/5] Fix failing snapshot --- web-ui/src/pages/FeedbackRequestPage.jsx | 1 - 1 file changed, 1 deletion(-) diff --git a/web-ui/src/pages/FeedbackRequestPage.jsx b/web-ui/src/pages/FeedbackRequestPage.jsx index 2c8cce9d50..7692f4e14e 100644 --- a/web-ui/src/pages/FeedbackRequestPage.jsx +++ b/web-ui/src/pages/FeedbackRequestPage.jsx @@ -321,7 +321,6 @@ const FeedbackRequestPage = () => { }, [canProceed]); return ( - queryLoaded.current &&
Feedback Request for {requestee?.name}