Skip to content

Commit a463bcf

Browse files
alishaevnalishaevn
authored
Merge pull request #326 from scientist-softserv/325-unauthed-experience
325-unauthed-experience
2 parents ebd1801 + 69d3490 commit a463bcf

File tree

4 files changed

+90
-57
lines changed

4 files changed

+90
-57
lines changed

‎pages/_app.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@ import {
1919
import '../utils/theme/globals.scss'
2020

2121
const WebStore = ({ Component }) => {
22+
/**
23+
* the session will return undefined if it's still loading, null if the user is not logged in, or an object if the user is logged in.
24+
*/
2225
const { data: session } = useSession()
2326
const router = useRouter()
2427

‎pages/requests/[uuid].js‎

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -58,25 +58,22 @@ const Request = ({ session }) => {
5858
}
5959
}, [request, isLoadingPOs, accessToken, uuid])
6060

61+
/**
62+
* checking for the presence of a session has to come after all of the hooks so we don't violate the react-hooks/rules-of-hooks
63+
* rule. ref: https://legacy.reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
64+
* we return the loading state in two different locations because it's rendered based on separate conditions. when
65+
* isLoading is true because we don't have a user, it doesn't ever become false. that's why we were previously returning
66+
* the loading spinner indefinitely.
67+
* using a guard clause with an early return inside the api methods also violates the react-hooks/rules-of-hooks rule.
68+
*/
69+
if (session === undefined) return pageLoading
70+
if (session === null) return unauthorizedUser
71+
6172
const isLoading = isLoadingRequest || isLoadingSOWs || isLoadingFiles || isLoadingMessages
6273
const isError = isRequestError || isSOWError || isFilesError|| isMessagesError || isPOError
6374
const documents = (allSOWs) ? [...allSOWs, ...allPOs] : []
6475

65-
if (isLoading) return <Loading wrapperClass='item-page mt-5' />
66-
67-
if (!session) {
68-
return (
69-
<Notice
70-
addClass='my-5'
71-
alert={{
72-
body: ['Please log in to view this request.'],
73-
title: 'Unauthorized',
74-
variant: 'info'
75-
}}
76-
dismissible={false}
77-
/>
78-
)
79-
}
76+
if (isLoading) return pageLoading
8077

8178
// this error is a result of creating the request
8279
if (createRequestError) {
@@ -219,4 +216,18 @@ const Request = ({ session }) => {
219216
)
220217
}
221218

219+
const pageLoading = <Loading wrapperClass='item-page mt-5' />
220+
221+
const unauthorizedUser = (
222+
<Notice
223+
addClass='my-5'
224+
alert={{
225+
body: ['Please log in to view this request.'],
226+
title: 'Unauthorized',
227+
variant: 'info'
228+
}}
229+
dismissible={false}
230+
/>
231+
)
232+
222233
export default Request

‎pages/requests/index.js‎

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -22,23 +22,17 @@ const Requests = ({ session }) => {
2222
const isError = isAllRequestsError || isDefaultWareError
2323
const isLoading = isLoadingAllRequests || isLoadingDefaultWare
2424

25-
// Check whether the user is authenticated first. If it does, we can return the API errors if applicable.
26-
27-
if (isLoading) return <Loading wrapperClass='mt-5' />
28-
29-
if (!session) {
30-
return (
31-
<Notice
32-
addClass='my-5'
33-
alert={{
34-
body: ['Please log in to make new requests or view existing ones.'],
35-
title: 'Unauthorized',
36-
variant: 'info'
37-
}}
38-
dismissible={false}
39-
/>
40-
)
41-
}
25+
/**
26+
* checking for the presence of a session has to come after all of the hooks so we don't violate the react-hooks/rules-of-hooks
27+
* rule. ref: https://legacy.reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
28+
* we return the loading state in two different locations because it's rendered based on separate conditions. when
29+
* isLoading is true because we don't have a user, it doesn't ever become false. that's why we were previously returning
30+
* the loading spinner indefinitely.
31+
* using a guard clause with an early return inside the api methods also violates the react-hooks/rules-of-hooks rule.
32+
*/
33+
if (session === undefined) return pageLoading
34+
if (session === null) return unauthorizedUser
35+
if (isLoading) return pageLoading
4236

4337
if (isError) {
4438
return (
@@ -72,4 +66,18 @@ const Requests = ({ session }) => {
7266
)
7367
}
7468

69+
const pageLoading = <Loading wrapperClass='mt-5' />
70+
71+
const unauthorizedUser = (
72+
<Notice
73+
addClass='my-5'
74+
alert={{
75+
body: ['Please log in to make new requests or view existing ones.'],
76+
title: 'Unauthorized',
77+
variant: 'info'
78+
}}
79+
dismissible={false}
80+
/>
81+
)
82+
7583
export default Requests

‎pages/requests/new/[ware].js‎

Lines changed: 36 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,27 @@ const NewRequest = ({ session }) => {
6161
const [buttonDisabled, setButtonDisabled] = useState(false)
6262
const [formSubmitting, setFormSubmitting] = useState(false)
6363

64+
useEffect(() => {
65+
if (createdRequestUUID) {
66+
router.push({
67+
pathname: `/requests/${createdRequestUUID}`,
68+
query: { createRequestError: JSON.stringify(createRequestError) },
69+
}, `/requests/${createdRequestUUID}`)
70+
}
71+
// eslint-disable-next-line react-hooks/exhaustive-deps
72+
}, [createdRequestUUID, createRequestError])
73+
74+
/**
75+
* checking for the presence of a session has to come after all of the hooks so we don't violate the react-hooks/rules-of-hooks
76+
* rule. ref: https://legacy.reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
77+
* we return the loading state in two different locations because it's rendered based on separate conditions. when
78+
* isLoading is true because we don't have a user, it doesn't ever become false. that's why we were previously returning
79+
* the loading spinner indefinitely.
80+
* using a guard clause with an early return inside the api methods also violates the react-hooks/rules-of-hooks rule.
81+
*/
82+
if (session === undefined) return pageLoading
83+
if (session === null) return unauthorizedUser
84+
6485
/**
6586
* @param {object} event onChange event
6687
* @param {string} property dot notated string representing the property in initialValue
@@ -118,32 +139,8 @@ const NewRequest = ({ session }) => {
118139
setCreatedRequestUUID(data.uuid)
119140
}
120141

121-
useEffect(() => {
122-
if (createdRequestUUID) {
123-
router.push({
124-
pathname: `/requests/${createdRequestUUID}`,
125-
query: { createRequestError: JSON.stringify(createRequestError) },
126-
}, `/requests/${createdRequestUUID}`)
127-
}
128-
// eslint-disable-next-line react-hooks/exhaustive-deps
129-
}, [createdRequestUUID, createRequestError])
130-
131142
// TODO(alishaevn): use react bs placeholder component
132-
if (isLoadingInitialRequest || !wareID || formSubmitting) return <Loading wrapperClass='item-page mt-5' />
133-
134-
if (!session) {
135-
return (
136-
<Notice
137-
addClass='my-5'
138-
alert={{
139-
body: ['Please log in to make new requests.'],
140-
title: 'Unauthorized',
141-
variant: 'info'
142-
}}
143-
dismissible={false}
144-
/>
145-
)
146-
}
143+
if (isLoadingInitialRequest || !wareID || formSubmitting) return pageLoading
147144

148145
if (isInitialRequestError) {
149146
return (
@@ -242,4 +239,18 @@ const StandardRequestOptions = ({ buttonDisabled, defaultRequiredDate, requestFo
242239
)
243240
}
244241

242+
const pageLoading = <Loading wrapperClass='item-page mt-5' />
243+
244+
const unauthorizedUser = (
245+
<Notice
246+
addClass='my-5'
247+
alert={{
248+
body: ['Please log in to make new requests.'],
249+
title: 'Unauthorized',
250+
variant: 'info'
251+
}}
252+
dismissible={false}
253+
/>
254+
)
255+
245256
export default NewRequest

0 commit comments

Comments
 (0)