Skip to content
Jump to bottom

Feature handle api status #220

Merged
merged 8 commits into from
Jul 14, 2021
+64 −22
Merged

Feature handle api status #220

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
74d139c
Fixed missing bracket
wrigh393 Jul 12, 2021
dc4a4cf
Implemented code for getting response info when error occurs
wrigh393 Jul 12, 2021
1931506
Implemented UI for showing error code and status message on failed AP…
wrigh393 Jul 12, 2021
305d694
Added appropriate icon for error UI; reformatted error UI text to emp…
wrigh393 Jul 14, 2021
1fad3bd
Remove unused state variable
campb303 Jul 14, 2021
759b164
Revert "Remove unused state variable"
campb303 Jul 14, 2021
99e8086
Refactor error message into its own component and cleanup ternary
campb303 Jul 14, 2021
861aea2
Refactor error message state variable to support dynamic error messag…
campb303 Jul 14, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 46 additions & 23 deletions src/components/AppView/AppView.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React, { useState, useEffect } from "react";
import PropTypes from "prop-types";
import { Box, makeStyles, Paper, useTheme } from "@material-ui/core";
import { Box, makeStyles, Paper, Typography, useTheme } from "@material-ui/core";
import { Route } from "react-router-dom";
import clsx from "clsx";
import ItemTableAppBar from "../ItemTableAppBar/";
Expand All @@ -10,46 +10,48 @@ import ItemView from "../ItemView/";
import QueueSelector from "../QueueSelector/";
import { useToken } from "../AuthProvider/";

export default function AppView({ setDarkMode }){
export default function AppView({ setDarkMode }) {
// Create stateful variables.
const [sidebarOpen, setSidebarOpen] = useState(false);
const [queues, setQueues] = useState([]);
const [items, setItems] = useState([]);
const [selectedQueues, setSelectedQueues] = useState([]);
const [queueSelectorOpen, setQueueSelectorOpen] = useState(false);
const [isLoading, setIsLoading] = useState(false);
const [error, setError] = useState(false)
const [response, setResponse] = useState({ status: '', message: '' })
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generic variable name. Replace with errorMessage and set to an initial value of empty string. Populate the message later if need be.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clarify, do you mean to replace both of these lines with one variable?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After sleeping on it, methinks line 22 can be nixed.


const access_token = useToken();

// Get Queues from API.
useEffect( _ => {
(async function getQueues(){
if (access_token === null){
useEffect(_ => {
(async function getQueues() {
if (access_token === null) {
return undefined;
}

if (queueSelectorOpen){
if (queueSelectorOpen) {
return undefined;
}

if (selectedQueues.length === 0){
if (selectedQueues.length === 0) {
setQueues([])
return undefined;
}

setIsLoading(true);
let queuesToLoad = "";

if (selectedQueues.length === 1){
if (selectedQueues.length === 1) {
queuesToLoad = selectedQueues[0].name;
}
else {
selectedQueues.forEach( (queue, index) => (
selectedQueues.forEach((queue, index) => (
index === 0
? queuesToLoad += queue.name
: queuesToLoad += `+${queue.name}`
));
}
}

let myHeaders = new Headers();
myHeaders.append("Authorization", `Bearer ${access_token}`);
Expand All @@ -58,15 +60,20 @@ export default function AppView({ setDarkMode }){
const apiResponse = await fetch(`${process.env.PUBLIC_URL}/api/data/${queuesToLoad}`, requestOptions);
const queueJson = await apiResponse.json();

setQueues(queueJson);
setIsLoading(false)
if (apiResponse.ok) {
setQueues(queueJson);
setIsLoading(false)
} else {
setResponse({ status: apiResponse.status, message: apiResponse.statusText })
setError(true)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code checks for errors after a bad request is received. This should run before line 61. It should also be replaced with a guard statement fo if (!apiResponse.ok) {...} that handles errors and returns. Code to follow should be what happens without error and not inside the else condition.

})();
}, [selectedQueues, access_token, queueSelectorOpen]);

// Populate items array.
useEffect( _ => {
useEffect(_ => {
let tempItems = [];
for (let queue of queues){
for (let queue of queues) {
tempItems = tempItems.concat(queue.items);
}
setItems(tempItems);
Expand Down Expand Up @@ -102,34 +109,50 @@ export default function AppView({ setDarkMode }){
width: "40vw",
}
},
errorContainer: {
display: "flex",
flexDirection: "column",
alignItems: "center",
justifyContent: "center",
marginTop: theme.spacing(6)
}
});
const classes = useStyles();

return(
return (
<Box component={Paper} display="flex" square elevation={0}>
<Box className={classes.leftCol}>
<ItemTableAppBar title="webqueue2" setDarkMode={setDarkMode} />
<QueueSelector
<QueueSelector
open={queueSelectorOpen}
setOpen={setQueueSelectorOpen}
value={selectedQueues}
setValue={setSelectedQueues}
/>
<ItemTable data={items} rowCanBeSelected={sidebarOpen} loading={isLoading}/>
/>{error ? <Box classes={{ root: classes.errorContainer }}>
<Typography variant="h1">
{response.status}
</Typography>
<Typography variant="h3">
Something went wrong.
</Typography>
<Typography variant="h5">
{response.message}
</Typography>
</Box> : <ItemTable data={items} rowCanBeSelected={sidebarOpen} loading={isLoading} />}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The information hierarchy emphasises the wrong information. On error, the API will send a JSON object with a message key and value describing the error. Replace this text with an appropriate icon and text below with the format of response.message + "\n" + "Error Code: " + response.status similar to: error message

</Box>
<Box className={clsx(classes.rightCol, sidebarOpen && classes.rightColShift)}>
{items.length === 0 ? null :
<Route
path="/:queue/:number"
render={({ match }) => (
<>
<ItemViewAppBar
title={`${match.params.queue} ${match.params.number}`}
<ItemViewAppBar
title={`${match.params.queue} ${match.params.number}`}
setSidebarOpen={setSidebarOpen}
/>
<ItemView
queue={match.params.queue}
number={match.params.number}
<ItemView
queue={match.params.queue}
number={match.params.number}
/>
</>
)}
Expand Down
2 changes: 1 addition & 1 deletion src/components/ItemTable/ItemTable.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export default function ItemTable({ data, rowCanBeSelected, loading }) {
{ Header: 'Last Updated', accessor: 'last_updated', sortInverted: true },
{ Header: 'Department', accessor: 'department' },
{ Header: 'Building', accessor: 'building' },
{ Header: 'Date Received', accessor: 'date_received', sortInverted: true ,
{ Header: 'Date Received', accessor: 'date_received', sortInverted: true , }
], []);
const tableInstance = useTable(
{
Expand Down